fix(settings): normalize legacy skip modes across persistence boundaries

This commit is contained in:
edde746
2026-09-06 03:54:43 +02:00
parent 7aa1679036
commit dca8029a99
4 changed files with 444 additions and 20 deletions
+64 -15
View File
@@ -54,8 +54,9 @@ class _PendingImport {
final String targetKey;
final String type;
final Object? value;
final String? obsoleteKey;
const _PendingImport({required this.targetKey, required this.type, required this.value});
const _PendingImport({required this.targetKey, required this.type, required this.value, this.obsoleteKey});
}
class _StoredPreferenceValue {
@@ -96,6 +97,10 @@ class SettingsExportService {
for (final pref in SettingsService.portablePrefs) pref.key: _PreferencePolicy(_storageTypeFor(pref)),
};
static final Map<String, String> _obsoleteSkipMarkerKeys = {
for (final entry in SettingsService.legacySkipMarkerPrefs.entries) entry.value.key: entry.key,
};
static const Set<String> _jsonStringListPreferenceKeys = {'hidden_libraries', 'library_order'};
static const Map<String, _PreferencePolicy> _userScopedPreferences = {
@@ -181,6 +186,17 @@ class SettingsExportService {
if (entry != null) prefsOut[baseKey] = entry;
}
// A snapshot must preserve cold-upgrade choices without mutating storage
// or depending on which typed preferences have been read by the UI.
for (final entry in SettingsService.legacySkipMarkerPrefs.entries) {
final pref = entry.value;
if (prefs.containsKey(pref.key)) continue;
final legacyValue = prefs.get(entry.key);
if (legacyValue is bool) {
prefsOut[pref.key] = {'type': _typeString, 'value': pref.fromLegacy(legacyValue).name};
}
}
return {
'formatVersion': formatVersion,
'appVersion': appVersion,
@@ -211,9 +227,10 @@ class SettingsExportService {
/// Applies a parsed export map to [prefs]. Pure and testable.
///
/// Each key in the import overwrites whatever value currently exists at the
/// same (possibly re-scoped) key. Keys not present in the import are left
/// alone — this is a per-key replacement, not a global wipe.
/// Each logical preference in the import replaces its target value, retiring
/// any obsolete representation. Omitted preferences are left alone. If both
/// representations occur, the canonical entry wins regardless of entry order;
/// an invalid canonical entry is skipped rather than replaced by a legacy one.
///
/// Throws [SettingsExportException] for structural problems.
static Future<ImportResult> applyImportMap(
@@ -239,16 +256,33 @@ class SettingsExportService {
int skipped = 0;
for (final entry in rawPrefs.entries) {
final baseKey = entry.key.toString();
final policy = _policyFor(baseKey);
var baseKey = entry.key.toString();
final rawEntry = entry.value;
if (policy == null || rawEntry is! Map) {
if (rawEntry is! Map) {
skipped++;
continue;
}
var type = rawEntry['type'];
var value = rawEntry['value'];
final legacyPref = SettingsService.legacySkipMarkerPrefs[baseKey];
if (version == 1 && legacyPref != null) {
if (rawPrefs.containsKey(legacyPref.key)) {
skipped++;
continue;
}
if (type == _typeBool && value is bool) {
baseKey = legacyPref.key;
type = _typeString;
value = legacyPref.fromLegacy(value).name;
}
}
final policy = _policyFor(baseKey);
if (policy == null) {
skipped++;
continue;
}
// Early format-v1 exports described these JSON-backed values as native
// string lists. Normalize that narrowly admitted legacy shape to the
// String representation consumed by StorageService.
@@ -269,22 +303,37 @@ class SettingsExportService {
}
pending.add(
_PendingImport(targetKey: policy.userScoped ? '$userPrefix$baseKey' : baseKey, type: type, value: value),
_PendingImport(
targetKey: policy.userScoped ? '$userPrefix$baseKey' : baseKey,
type: type,
value: value,
obsoleteKey: _obsoleteSkipMarkerKeys[baseKey],
),
);
}
final snapshots = <String, _StoredPreferenceValue>{
for (final mutation in pending)
mutation.targetKey: _StoredPreferenceValue(
existed: prefs.keys.contains(mutation.targetKey),
value: prefs.get(mutation.targetKey),
),
};
final snapshots = <String, _StoredPreferenceValue>{};
for (final mutation in pending) {
snapshots[mutation.targetKey] = _StoredPreferenceValue(
existed: prefs.containsKey(mutation.targetKey),
value: prefs.get(mutation.targetKey),
);
if (mutation.obsoleteKey case final obsoleteKey?) {
snapshots[obsoleteKey] = _StoredPreferenceValue(
existed: prefs.containsKey(obsoleteKey),
value: prefs.get(obsoleteKey),
);
}
}
try {
for (final mutation in pending) {
await debugBeforeImportWrite?.call(mutation.targetKey);
await _writeTyped(prefs, mutation.targetKey, mutation.type, mutation.value);
if (mutation.obsoleteKey case final obsoleteKey?) {
await debugBeforeImportWrite?.call(obsoleteKey);
await prefs.remove(obsoleteKey);
}
}
} catch (error, stackTrace) {
try {
+28 -4
View File
@@ -260,13 +260,21 @@ class _SkipMarkerModePref extends EnumPref<SkipMarkerMode> {
const _SkipMarkerModePref(super.key, {required this.legacyKey})
: super(values: SkipMarkerMode.values, defaultValue: SkipMarkerMode.button);
SkipMarkerMode fromLegacy(bool value) => value ? SkipMarkerMode.auto : SkipMarkerMode.button;
@override
SkipMarkerMode readFrom(BaseSharedPreferencesService svc) {
// A committed mode is authoritative, including one restored before this
// preference's first read. A leftover boolean must never replace it.
if (svc.prefs.containsKey(key)) {
if (svc.prefs.containsKey(legacyKey)) svc.prefs.remove(legacyKey);
return super.readFrom(svc);
}
final legacyValue = svc.readNullableBool(legacyKey);
if (legacyValue != null) {
final migrated = legacyValue ? SkipMarkerMode.auto : SkipMarkerMode.button;
svc.prefs.remove(legacyKey);
final migrated = fromLegacy(legacyValue);
svc.prefs.setString(key, migrated.name);
svc.prefs.remove(legacyKey);
return migrated;
}
return super.readFrom(svc);
@@ -524,6 +532,14 @@ class SettingsService extends BaseSharedPreferencesService {
static const clickVideoTogglesPlayback = BoolPref('click_video_toggles_playback');
static const skipIntroMode = _SkipMarkerModePref('skip_intro_mode', legacyKey: _legacyAutoSkipIntroKey);
static const skipCreditsMode = _SkipMarkerModePref('skip_credits_mode', legacyKey: _legacyAutoSkipCreditsKey);
/// Previous format-v1 representations of these logical preferences.
/// Shared by the typed migration and portable settings codec.
static const legacySkipMarkerPrefs = {
_legacyAutoSkipIntroKey: skipIntroMode,
_legacyAutoSkipCreditsKey: skipCreditsMode,
};
static const forceSkipMarkerFallback = BoolPref('force_skip_marker_fallback');
static const autoSkipDelay = IntPref('auto_skip_delay', defaultValue: 5);
static const introPattern = StringPref('intro_pattern', defaultValue: defaultIntroPattern);
@@ -1303,12 +1319,20 @@ class SettingsService extends BaseSharedPreferencesService {
static List<Pref<Object?>> get portablePrefs => [..._resetAndPortablePrefs, ..._portableOnlyPrefs];
Future<void> resetAllSettings() async {
// Marker modes are portable-only. Preserve cold-upgrade choices before
// retiring their old representation, just as if they had already been read.
for (final entry in legacySkipMarkerPrefs.entries) {
final pref = entry.value;
if (!prefs.containsKey(pref.key)) {
final legacyValue = readNullableBool(entry.key);
if (legacyValue != null) await pref.writeTo(this, pref.fromLegacy(legacyValue));
}
await prefs.remove(entry.key);
}
await Future.wait([
..._resettablePrefs.map((p) => prefs.remove(p.key)),
// Legacy migration sentinels — removed alongside the keys they guarded.
prefs.remove(_legacyUseSeasonPosterKey),
prefs.remove(_legacyAutoSkipIntroKey),
prefs.remove(_legacyAutoSkipCreditsKey),
prefs.remove(_legacyMpvConfigEntriesKey),
prefs.remove(_legacyBufferSizeKey),
prefs.remove(_legacyDemuxerModeKey),
@@ -199,6 +199,281 @@ void main() {
});
});
group('skip marker backup compatibility', () {
for (final introAuto in [true, false]) {
test('exports legacy intro=$introAuto identically before and after typed reads', () async {
resetSharedPreferencesForTest(initialAsync: {'auto_skip_intro': introAuto, 'auto_skip_credits': !introAuto});
final prefs = await BaseSharedPreferencesService.sharedCache();
final expected = {
'skip_intro_mode': {'type': 'string', 'value': introAuto ? 'auto' : 'button'},
'skip_credits_mode': {'type': 'string', 'value': introAuto ? 'button' : 'auto'},
};
expect(SettingsExportService.buildExportMap(prefs)['prefs'], expected);
expect(prefs.getBool('auto_skip_intro'), introAuto);
expect(prefs.getBool('auto_skip_credits'), !introAuto);
final settings = await SettingsService.getInstance();
expect(settings.read(SettingsService.skipIntroMode), introAuto ? SkipMarkerMode.auto : SkipMarkerMode.button);
expect(SettingsExportService.buildExportMap(prefs)['prefs'], expected);
expect(settings.read(SettingsService.skipCreditsMode), introAuto ? SkipMarkerMode.button : SkipMarkerMode.auto);
expect(SettingsExportService.buildExportMap(prefs)['prefs'], expected);
});
for (final target in ['empty', 'legacy', 'canonical']) {
test('restores a 2.18.0 intro=$introAuto backup over a $target target', () async {
resetSharedPreferencesForTest(
initialAsync: {
if (target == 'legacy') ...{'auto_skip_intro': !introAuto, 'auto_skip_credits': introAuto},
if (target == 'canonical') ...{
'skip_intro_mode': introAuto ? 'off' : 'auto',
'skip_credits_mode': introAuto ? 'auto' : 'off',
},
'seek_time_small': 45,
},
);
final prefs = await BaseSharedPreferencesService.sharedCache();
final result = await SettingsExportService.applyImportMap(
{
'formatVersion': 1,
'appVersion': '2.18.0',
'prefs': {
'auto_skip_intro': {'type': 'bool', 'value': introAuto},
'auto_skip_credits': {'type': 'bool', 'value': !introAuto},
},
},
prefs,
currentUserUuid: 'target-user',
);
final expected = {
'skip_intro_mode': {'type': 'string', 'value': introAuto ? 'auto' : 'button'},
'skip_credits_mode': {'type': 'string', 'value': introAuto ? 'button' : 'auto'},
'seek_time_small': {'type': 'int', 'value': 45},
};
expect(result.keysImported, 2);
expect(result.keysSkipped, 0);
expect(prefs.containsKey('auto_skip_intro'), isFalse);
expect(prefs.containsKey('auto_skip_credits'), isFalse);
expect(SettingsExportService.buildExportMap(prefs)['prefs'], expected);
var settings = await SettingsService.getInstance();
for (var read = 0; read < 2; read++) {
expect(
settings.read(SettingsService.skipIntroMode),
introAuto ? SkipMarkerMode.auto : SkipMarkerMode.button,
);
expect(
settings.read(SettingsService.skipCreditsMode),
introAuto ? SkipMarkerMode.button : SkipMarkerMode.auto,
);
}
BaseSharedPreferencesService.resetForTesting();
SettingsService.resetForTesting();
settings = await SettingsService.getInstance();
expect(settings.read(SettingsService.skipIntroMode), introAuto ? SkipMarkerMode.auto : SkipMarkerMode.button);
expect(
settings.read(SettingsService.skipCreditsMode),
introAuto ? SkipMarkerMode.button : SkipMarkerMode.auto,
);
expect(SettingsExportService.buildExportMap(settings.prefs)['prefs'], expected);
});
}
}
test('canonical imports replace unread legacy choices through restart and re-export', () async {
resetSharedPreferencesForTest(initialAsync: const {'auto_skip_intro': true, 'auto_skip_credits': false});
final prefs = await BaseSharedPreferencesService.sharedCache();
const expected = {
'skip_intro_mode': {'type': 'string', 'value': 'off'},
'skip_credits_mode': {'type': 'string', 'value': 'auto'},
};
await SettingsExportService.applyImportMap(
{'formatVersion': 1, 'prefs': expected},
prefs,
currentUserUuid: 'target-user',
);
expect(prefs.containsKey('auto_skip_intro'), isFalse);
expect(prefs.containsKey('auto_skip_credits'), isFalse);
expect(SettingsExportService.buildExportMap(prefs)['prefs'], expected);
var settings = await SettingsService.getInstance();
expect(settings.read(SettingsService.skipIntroMode), SkipMarkerMode.off);
expect(settings.read(SettingsService.skipCreditsMode), SkipMarkerMode.auto);
expect(SettingsExportService.buildExportMap(prefs)['prefs'], expected);
BaseSharedPreferencesService.resetForTesting();
SettingsService.resetForTesting();
settings = await SettingsService.getInstance();
expect(settings.read(SettingsService.skipIntroMode), SkipMarkerMode.off);
expect(settings.read(SettingsService.skipCreditsMode), SkipMarkerMode.auto);
expect(SettingsExportService.buildExportMap(settings.prefs)['prefs'], expected);
});
for (final canonicalFirst in [false, true]) {
test('canonical payload precedence is independent of order: first=$canonicalFirst', () async {
final prefs = await BaseSharedPreferencesService.sharedCache();
const canonical = {
'skip_intro_mode': {'type': 'string', 'value': 'off'},
'skip_credits_mode': {'type': 'string', 'value': 'auto'},
};
const legacy = {
'auto_skip_intro': {'type': 'bool', 'value': true},
'auto_skip_credits': {'type': 'bool', 'value': false},
};
await SettingsExportService.applyImportMap(
{
'formatVersion': 1,
'prefs': canonicalFirst ? {...canonical, ...legacy} : {...legacy, ...canonical},
},
prefs,
currentUserUuid: 'target-user',
);
final settings = await SettingsService.getInstance();
expect(settings.read(SettingsService.skipIntroMode), SkipMarkerMode.off);
expect(settings.read(SettingsService.skipCreditsMode), SkipMarkerMode.auto);
expect(SettingsExportService.buildExportMap(prefs)['prefs'], canonical);
});
}
test('export gives coexisting canonical values precedence before any migration', () async {
resetSharedPreferencesForTest(
initialAsync: const {
'auto_skip_intro': true,
'skip_intro_mode': 'off',
'auto_skip_credits': false,
'skip_credits_mode': 'auto',
},
);
final prefs = await BaseSharedPreferencesService.sharedCache();
const expected = {
'skip_intro_mode': {'type': 'string', 'value': 'off'},
'skip_credits_mode': {'type': 'string', 'value': 'auto'},
};
expect(SettingsExportService.buildExportMap(prefs)['prefs'], expected);
final settings = await SettingsService.getInstance();
expect(settings.read(SettingsService.skipIntroMode), SkipMarkerMode.off);
expect(settings.read(SettingsService.skipCreditsMode), SkipMarkerMode.auto);
expect(SettingsExportService.buildExportMap(prefs)['prefs'], expected);
});
test('omitted logical preferences and unrelated settings remain untouched', () async {
resetSharedPreferencesForTest(
initialAsync: const {'auto_skip_intro': true, 'auto_skip_credits': true, 'seek_time_small': 45},
);
final prefs = await BaseSharedPreferencesService.sharedCache();
await SettingsExportService.applyImportMap(
{
'formatVersion': 1,
'prefs': {
'auto_skip_intro': {'type': 'bool', 'value': false},
},
},
prefs,
currentUserUuid: 'target-user',
);
expect(prefs.getBool('auto_skip_credits'), isTrue);
expect(prefs.containsKey('skip_credits_mode'), isFalse);
expect(prefs.getInt('seek_time_small'), 45);
final settings = await SettingsService.getInstance();
expect(settings.read(SettingsService.skipIntroMode), SkipMarkerMode.button);
expect(settings.read(SettingsService.skipCreditsMode), SkipMarkerMode.auto);
});
test('invalid legacy entries neither migrate nor retire target choices', () async {
resetSharedPreferencesForTest(initialAsync: const {'auto_skip_intro': false, 'skip_credits_mode': 'off'});
final prefs = await BaseSharedPreferencesService.sharedCache();
final result = await SettingsExportService.applyImportMap(
{
'formatVersion': 1,
'prefs': {
'auto_skip_intro': {'type': 'bool', 'value': 'true'},
'auto_skip_credits': {'type': 'string', 'value': true},
},
},
prefs,
currentUserUuid: 'target-user',
);
expect(result.keysImported, 0);
expect(result.keysSkipped, 2);
expect(prefs.getBool('auto_skip_intro'), isFalse);
expect(prefs.containsKey('skip_intro_mode'), isFalse);
final settings = await SettingsService.getInstance();
expect(settings.read(SettingsService.skipIntroMode), SkipMarkerMode.button);
expect(settings.read(SettingsService.skipCreditsMode), SkipMarkerMode.off);
});
test('an invalid canonical entry does not fall through to a conflicting legacy entry', () async {
resetSharedPreferencesForTest(initialAsync: const {'skip_intro_mode': 'off'});
final prefs = await BaseSharedPreferencesService.sharedCache();
await SettingsExportService.applyImportMap(
{
'formatVersion': 1,
'prefs': {
'auto_skip_intro': {'type': 'bool', 'value': true},
'skip_intro_mode': {'type': 'bool', 'value': false},
},
},
prefs,
currentUserUuid: 'target-user',
);
final settings = await SettingsService.getInstance();
expect(settings.read(SettingsService.skipIntroMode), SkipMarkerMode.off);
});
for (final failAt in ['auto_skip_intro', 'skip_credits_mode']) {
test('rolls back both representations when mutation fails at $failAt', () async {
resetSharedPreferencesForTest(
initialAsync: {
'auto_skip_intro': true,
if (failAt == 'auto_skip_intro') 'skip_intro_mode': 'button',
'auto_skip_credits': false,
'seek_time_small': 45,
},
);
final prefs = await BaseSharedPreferencesService.sharedCache();
final before = {for (final key in prefs.keys) key: prefs.get(key)};
final exportedBefore = SettingsExportService.buildExportMap(prefs)['prefs'];
SettingsExportService.debugBeforeImportWrite = (key) {
if (key == failAt) throw StateError('synthetic write failure');
};
await expectLater(
SettingsExportService.applyImportMap(
{
'formatVersion': 1,
'prefs': {
'skip_intro_mode': {'type': 'string', 'value': 'off'},
'auto_skip_credits': {'type': 'bool', 'value': true},
},
},
prefs,
currentUserUuid: 'target-user',
),
throwsA(isA<StateError>()),
);
expect({for (final key in prefs.keys) key: prefs.get(key)}, before);
expect(SettingsExportService.buildExportMap(prefs)['prefs'], exportedBefore);
BaseSharedPreferencesService.resetForTesting();
SettingsService.resetForTesting();
final restarted = await BaseSharedPreferencesService.sharedCache();
expect({for (final key in restarted.keys) key: restarted.get(key)}, before);
expect(SettingsExportService.buildExportMap(restarted)['prefs'], exportedBefore);
final settings = await SettingsService.getInstance();
expect(
settings.read(SettingsService.skipIntroMode),
failAt == 'auto_skip_intro' ? SkipMarkerMode.button : SkipMarkerMode.auto,
);
expect(settings.read(SettingsService.skipCreditsMode), SkipMarkerMode.button);
});
}
});
group('transactional import', () {
test('validates version and structure before writing', () async {
final prefs = await BaseSharedPreferencesService.sharedCache();
+77 -1
View File
@@ -149,10 +149,86 @@ void main() {
expect(settings.prefs.containsKey('auto_skip_intro'), isFalse, reason: 'migrated once, then forgotten');
expect(settings.prefs.containsKey('auto_skip_credits'), isFalse);
// A later explicit choice is not clobbered by a stale legacy key.
// An explicit choice still works after ordinary upgrade migration.
await settings.write(SettingsService.skipIntroMode, SkipMarkerMode.off);
expect(settings.read(SettingsService.skipIntroMode), SkipMarkerMode.off);
});
test('canonical modes win over coexisting legacy booleans on first read', () async {
resetSharedPreferencesForTest(
initialAsync: const {
'auto_skip_intro': true,
'skip_intro_mode': 'off',
'auto_skip_credits': false,
'skip_credits_mode': 'auto',
},
);
var settings = await SettingsService.getInstance();
expect(settings.read(SettingsService.skipIntroMode), SkipMarkerMode.off);
expect(settings.read(SettingsService.skipCreditsMode), SkipMarkerMode.auto);
expect(settings.prefs.containsKey('auto_skip_intro'), isFalse);
expect(settings.prefs.containsKey('auto_skip_credits'), isFalse);
BaseSharedPreferencesService.resetForTesting();
SettingsService.resetForTesting();
settings = await SettingsService.getInstance();
expect(settings.read(SettingsService.skipIntroMode), SkipMarkerMode.off);
expect(settings.read(SettingsService.skipCreditsMode), SkipMarkerMode.auto);
});
test('writing a mode before its first read replaces the legacy choice', () async {
resetSharedPreferencesForTest(initialAsync: const {'auto_skip_intro': true, 'auto_skip_credits': false});
final settings = await SettingsService.getInstance();
await settings.write(SettingsService.skipIntroMode, SkipMarkerMode.off);
await settings.write(SettingsService.skipCreditsMode, SkipMarkerMode.auto);
expect(settings.read(SettingsService.skipIntroMode), SkipMarkerMode.off);
expect(settings.read(SettingsService.skipCreditsMode), SkipMarkerMode.auto);
});
for (final introAuto in [true, false]) {
for (final readBeforeReset in [false, true]) {
test('reset preserves intro=$introAuto before migration=${!readBeforeReset}', () async {
resetSharedPreferencesForTest(
initialAsync: {'auto_skip_intro': introAuto, 'auto_skip_credits': !introAuto, 'seek_time_small': 45},
);
var settings = await SettingsService.getInstance();
final introMode = introAuto ? SkipMarkerMode.auto : SkipMarkerMode.button;
final creditsMode = introAuto ? SkipMarkerMode.button : SkipMarkerMode.auto;
if (readBeforeReset) {
expect(settings.read(SettingsService.skipIntroMode), introMode);
expect(settings.read(SettingsService.skipCreditsMode), creditsMode);
}
await settings.resetAllSettings();
expect(settings.read(SettingsService.skipIntroMode), introMode);
expect(settings.read(SettingsService.skipCreditsMode), creditsMode);
expect(settings.read(SettingsService.seekTimeSmall), 10);
BaseSharedPreferencesService.resetForTesting();
SettingsService.resetForTesting();
settings = await SettingsService.getInstance();
expect(settings.read(SettingsService.skipIntroMode), introMode);
expect(settings.read(SettingsService.skipCreditsMode), creditsMode);
});
}
}
test('reset preserves canonical authority without materializing an absent mode', () async {
resetSharedPreferencesForTest(initialAsync: const {'skip_intro_mode': 'off', 'auto_skip_intro': true});
final settings = await SettingsService.getInstance();
await settings.resetAllSettings();
expect(settings.read(SettingsService.skipIntroMode), SkipMarkerMode.off);
expect(settings.prefs.containsKey('auto_skip_intro'), isFalse);
expect(settings.prefs.containsKey('skip_credits_mode'), isFalse);
expect(settings.read(SettingsService.skipCreditsMode), SkipMarkerMode.button);
expect(settings.prefs.containsKey('skip_credits_mode'), isFalse);
});
});
group('SettingsService music quality', () {