Skip to content

Commit

Permalink
[shared_preferences] Adds allowList to setPrefix method. (#3794)
Browse files Browse the repository at this point in the history
Creates optional allow list for preference keys when setting custom prefix.

Also makes integration tests more homogenous across platforms. 

Fixes flutter/flutter#128948
  • Loading branch information
tarrinneal authored Jun 30, 2023
1 parent 8522793 commit 03c73a6
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 20 deletions.
4 changes: 4 additions & 0 deletions packages/shared_preferences/shared_preferences/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.2.0

* Adds `allowList` option to setPrefix.

## 2.1.2

* Fixes singleton initialization race condition introduced during NNBD
Expand Down
1 change: 1 addition & 0 deletions packages/shared_preferences/shared_preferences/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ by any non-flutter versions of the app (for migrating from a native app to flutt
If the prefix is set to a value such as `''` that causes it to read values that were
not originally stored by the `SharedPreferences`, initializing `SharedPreferences`
may fail if any of the values are of types that are not supported by `SharedPreferences`.
In this case, you can set an `allowList` that contains only preferences of supported types.

If you decide to remove the prefix entirely, you can still access previously created
preferences by manually adding the previous prefix `flutter.` to the beginning of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ void main() {
preferences = await SharedPreferences.getInstance();
});

tearDown(() {
preferences.clear();
tearDown(() async {
await preferences.clear();
SharedPreferences.resetStatic();
});

Expand All @@ -111,8 +111,8 @@ void main() {
preferences = await SharedPreferences.getInstance();
});

tearDown(() {
preferences.clear();
tearDown(() async {
await preferences.clear();
SharedPreferences.resetStatic();
});

Expand All @@ -126,11 +126,40 @@ void main() {
preferences = await SharedPreferences.getInstance();
});

tearDown(() {
preferences.clear();
tearDown(() async {
await preferences.clear();
SharedPreferences.resetStatic();
});

runAllTests();
});

testWidgets('allowList only gets allowed items', (WidgetTester _) async {
const String allowedString = 'stringKey';
const String allowedBool = 'boolKey';
const String notAllowedDouble = 'doubleKey';
const String resultString = 'resultString';

const Set<String> allowList = <String>{allowedString, allowedBool};

SharedPreferences.resetStatic();
SharedPreferences.setPrefix('', allowList: allowList);

final SharedPreferences prefs = await SharedPreferences.getInstance();

await prefs.setString(allowedString, resultString);
await prefs.setBool(allowedBool, true);
await prefs.setDouble(notAllowedDouble, 3.14);

await prefs.reload();

final String? testString = prefs.getString(allowedString);
expect(testString, resultString);

final bool? testBool = prefs.getBool(allowedBool);
expect(testBool, true);

final double? testDouble = prefs.getDouble(notAllowedDouble);
expect(testDouble, null);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:async';

import 'package:flutter/foundation.dart' show visibleForTesting;
import 'package:shared_preferences_platform_interface/shared_preferences_platform_interface.dart';
import 'package:shared_preferences_platform_interface/types.dart';

/// Wraps NSUserDefaults (on iOS) and SharedPreferences (on Android), providing
/// a persistent store for simple data.
Expand All @@ -18,6 +19,8 @@ class SharedPreferences {

static bool _prefixHasBeenChanged = false;

static Set<String>? _allowList;

static Completer<SharedPreferences>? _completer;

static SharedPreferencesStorePlatform get _store =>
Expand All @@ -33,17 +36,23 @@ class SharedPreferences {
/// previous behavior of this plugin. To use preferences with no prefix,
/// set [prefix] to ''.
///
/// If [prefix] is set to '', you may encounter preferences that are
/// incompatible with shared_preferences. The optional parameter
/// [allowList] will cause the plugin to only return preferences that
/// are both contained in the list AND match the provided prefix.
///
/// No migration of existing preferences is performed by this method.
/// If you set a different prefix, and have previously stored preferences,
/// you will need to handle any migration yourself.
///
/// This cannot be called after `getInstance`.
static void setPrefix(String prefix) {
static void setPrefix(String prefix, {Set<String>? allowList}) {
if (_completer != null) {
throw StateError('setPrefix cannot be called after getInstance');
}
_prefix = prefix;
_prefixHasBeenChanged = true;
_allowList = allowList;
}

/// Resets class's static values to allow for testing of setPrefix flow.
Expand All @@ -52,6 +61,7 @@ class SharedPreferences {
_completer = null;
_prefix = 'flutter.';
_prefixHasBeenChanged = false;
_allowList = null;
}

/// Loads and parses the [SharedPreferences] for this app from disk.
Expand Down Expand Up @@ -182,8 +192,14 @@ class SharedPreferences {
_preferenceCache.clear();
if (_prefixHasBeenChanged) {
try {
// ignore: deprecated_member_use
return _store.clearWithPrefix(_prefix);
return _store.clearWithParameters(
ClearParameters(
filter: PreferencesFilter(
prefix: _prefix,
allowList: _allowList,
),
),
);
} catch (e) {
// Catching and clarifying UnimplementedError to provide a more robust message.
if (e is UnimplementedError) {
Expand Down Expand Up @@ -214,8 +230,16 @@ Either update the implementation to support setPrefix, or do not call setPrefix.
final Map<String, Object> fromSystem = <String, Object>{};
if (_prefixHasBeenChanged) {
try {
// ignore: deprecated_member_use
fromSystem.addAll(await _store.getAllWithPrefix(_prefix));
fromSystem.addAll(
await _store.getAllWithParameters(
GetAllParameters(
filter: PreferencesFilter(
prefix: _prefix,
allowList: _allowList,
),
),
),
);
} catch (e) {
// Catching and clarifying UnimplementedError to provide a more robust message.
if (e is UnimplementedError) {
Expand Down
4 changes: 2 additions & 2 deletions packages/shared_preferences/shared_preferences/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: Flutter plugin for reading and writing simple key-value pairs.
Wraps NSUserDefaults on iOS and SharedPreferences on Android.
repository: https://github.com/flutter/packages/tree/main/packages/shared_preferences/shared_preferences
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+shared_preferences%22
version: 2.1.2
version: 2.2.0

environment:
sdk: ">=2.18.0 <4.0.0"
Expand Down Expand Up @@ -31,7 +31,7 @@ dependencies:
shared_preferences_android: ^2.1.0
shared_preferences_foundation: ^2.2.0
shared_preferences_linux: ^2.2.0
shared_preferences_platform_interface: ^2.2.0
shared_preferences_platform_interface: ^2.3.0
shared_preferences_web: ^2.1.0
shared_preferences_windows: ^2.2.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:shared_preferences/shared_preferences.dart';
import 'package:shared_preferences_platform_interface/shared_preferences_platform_interface.dart';
import 'package:shared_preferences_platform_interface/types.dart';

void main() {
TestWidgetsFlutterBinding.ensureInitialized();
Expand Down Expand Up @@ -256,6 +257,30 @@ void main() {
expect(testDouble, 3.14);
});

test('allowList only gets allowed items', () async {
const Set<String> allowList = <String>{'stringKey', 'boolKey'};

SharedPreferences.resetStatic();
SharedPreferences.setPrefix('', allowList: allowList);

final SharedPreferences prefs = await SharedPreferences.getInstance();

await prefs.setString('stringKey', 'test');
await prefs.setBool('boolKey', true);
await prefs.setDouble('doubleKey', 3.14);

await prefs.reload();

final String? testString = prefs.getString('stringKey');
expect(testString, 'test');

final bool? testBool = prefs.getBool('boolKey');
expect(testBool, true);

final double? testDouble = prefs.getDouble('doubleKey');
expect(testDouble, null);
});

test('using reload after setPrefix properly reloads the cache', () async {
const String newPrefix = 'newPrefix';

Expand All @@ -274,7 +299,7 @@ void main() {
expect(testStrings, 'test');
});

test('unimplemented errors in withPrefix methods are updated', () async {
test('unimplemented errors in withParameters methods are updated', () async {
final UnimplementedSharedPreferencesStore localStore =
UnimplementedSharedPreferencesStore();
SharedPreferencesStorePlatform.instance = localStore;
Expand All @@ -294,7 +319,7 @@ void main() {
"Shared Preferences doesn't yet support the setPrefix method"));
});

test('non-Unimplemented errors pass through withPrefix methods correctly',
test('non-Unimplemented errors pass through withParameters methods correctly',
() async {
final ThrowingSharedPreferencesStore localStore =
ThrowingSharedPreferencesStore();
Expand Down Expand Up @@ -326,17 +351,23 @@ class FakeSharedPreferencesStore extends SharedPreferencesStorePlatform {
return backend.clear();
}

@override
Future<bool> clearWithParameters(ClearParameters parameters) {
log.add(const MethodCall('clearWithParameters'));
return backend.clearWithParameters(parameters);
}

@override
Future<Map<String, Object>> getAll() {
log.add(const MethodCall('getAll'));
return backend.getAll();
}

@override
Future<Map<String, Object>> getAllWithPrefix(String prefix) {
log.add(const MethodCall('getAllWithPrefix'));
// ignore: deprecated_member_use
return backend.getAllWithPrefix(prefix);
Future<Map<String, Object>> getAllWithParameters(
GetAllParameters parameters) {
log.add(const MethodCall('getAllWithParameters'));
return backend.getAllWithParameters(parameters);
}

@override
Expand Down Expand Up @@ -406,7 +437,8 @@ class ThrowingSharedPreferencesStore extends SharedPreferencesStorePlatform {
}

@override
Future<Map<String, Object>> getAllWithPrefix(String prefix) {
Future<Map<String, Object>> getAllWithParameters(
GetAllParameters parameters) {
throw StateError('State Error');
}
}

0 comments on commit 03c73a6

Please sign in to comment.