8000 [shared_preferences] Migrate platform plugins to null-safety by gaetschwartz · Pull Request #3523 · flutter/plugins · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[shared_preferences] Migrate platform plugins to null-safety #3523

Merged
merged 12 commits into from gaetschwartz:nnbd
Feb 8, 2021
Merged

[shared_preferences] Migrate platform plugins to null-safety #3523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2e22820
[shared_preferences] Migrate platform plugins to nnbd
gaetschwartz Feb 6, 2021
235cef0
Update CHANGELOGs
gaetschwartz Feb 6, 2021
e19321f
fixed dependency issues
gaetschwartz Feb 6, 2021
e2e33ec
fix fortmatting
gaetschwartz Feb 6, 2021
1119226
fix linux test
gaetschwartz Feb 6, 2021
dad45b8
fix windows example dependencies
gaetschwartz Feb 6, 2021
798a9b6
Fixed generated_plugin_registrant format
gaetschwartz Feb 6, 2021
6cf839c
Fix dependencies again + unnecessary nullable field
gaetschwartz Feb 6, 2021
c4acbf3
Fix file version + linux logic for _cached
gaetschwartz Feb 6, 2021
827e55f
Revert windows
gaetschwartz Feb 6, 2021
286637c
Fixes
gaetschwartz Feb 8, 2021
92577c9
Adjust read/write to match Windows
stuartmorgan Feb 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/shared_preferences/shared_preferences_linux/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.0.4-nullsafety

* Migrate to null-safety.

## 0.0.3+1

* Update Flutter SDK constraint.
Expand Down
87 changes: 47 additions & 40 deletions ...references/shared_preferences_linux/example/integration_test/shared_preferences_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import 'dart:async';

import 'package:flutter_test/flutter_test.dart';
import 'package:shared_preferences/shared_preferences.dart';
import 'package:integration_test/integration_test.dart';
import 'package:shared_preferences_linux/shared_preferences_linux.dart';

void main() {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();

group('$SharedPreferences', () {
group('SharedPreferencesLinux', () {
const Map<String, dynamic> kTestValues = <String, dynamic>{
'flutter.String': 'hello world',
'flutter.bool': true,
Expand All @@ -23,67 +24,73 @@ void main() {
'flutter.List': <String>['baz', 'quox'],
};

SharedPreferences preferences;
SharedPreferencesLinux preferences;

setUp(() async {
preferences = await SharedPreferences.getInstance();
preferences = SharedPreferencesLinux.instance;
});

tearDown(() {
preferences.clear();
});

testWidgets('reading', (WidgetTester _) async {
expect(preferences.get('String'), isNull);
expect(preferences.get('bool'), isNull);
expect(preferences.get('int'), isNull);
expect(preferences.get('double'), isNull);
expect(preferences.get('List'), isNull);
expect(preferences.getString('String'), isNull);
expect(preferences.getBool('bool'), isNull);
expect(preferences.getInt('int'), isNull);
expect(preferences.getDouble('double'), isNull);
expect(preferences.getStringList('List'), isNull);
final all = await preferences.getAll();
expect(all['String'], isNull);
expect(all['bool'], isNull);
expect(all['int'], isNull);
expect(all['double'], isNull);
expect(all['List'], isNull);
});

testWidgets('writing', (WidgetTester _) async {
await Future.wait(<Future<bool>>[
preferences.setString('String', kTestValues2['flutter.String']),
preferences.setBool('bool', kTestValues2['flutter.bool']),
preferences.setInt('int', kTestValues2['flutter.int']),
preferences.setDouble('double', kTestValues2['flutter.double']),
preferences.setStringList('List', kTestValues2['flutter.List'])
preferences.setValue(
'String', 'String', kTestValues2['flutter.String']),
preferences.setValue('Bool', 'bool', kTestValues2['flutter.bool']),
preferences.setValue('Int', 'int', kTestValues2['flutter.int']),
preferences.setValue(
'Double', 'double', kTestValues2['flutter.double']),
preferences.setValue('StringList', 'List', kTestValues2['flutter.List'])
]);
expect(preferences.getString('String'), kTestValues2['flutter.String']);
expect(preferences.getBool('bool'), kTestValues2['flutter.bool']);
expect(preferences.getInt('int'), kTestValues2['flutter.int']);
expect(preferences.getDouble('double'), kTestValues2['flutter.double']);
expect(preferences.getStringList('List'), kTestValues2['flutter.List']);
final all = await preferences.getAll();
expect(all['String'], kTestValues2['flutter.String']);
expect(all['bool'], kTestValues2['flutter.bool']);
expect(all['int'], kTestValues2['flutter.int']);
expect(all['double'], kTestValues2['flutter.double']);
expect(all['List'], kTestValues2['flutter.List']);
});

testWidgets('removing', (WidgetTester _) async {
const String key = 'testKey';
await preferences.setString(key, kTestValues['flutter.String']);
await preferences.setBool(key, kTestValues['flutter.bool']);
await preferences.setInt(key, kTestValues['flutter.int']);
await preferences.setDouble(key, kTestValues['flutter.double']);
await preferences.setStringList(key, kTestValues['flutter.List']);

await Future.wait([
preferences.setValue('String', key, kTestValues['flutter.String']),
preferences.setValue('Bool', key, kTestValues['flutter.bool']),
preferences.setValue('Int', key, kTestValues['flutter.int']),
preferences.setValue('Double', key, kTestValues['flutter.double']),
preferences.setValue('StringList', key, kTestValues['flutter.List'])
]);
await preferences.remove(key);
expect(preferences.get('testKey'), isNull);
final all = await preferences.getAll();
expect(all['testKey'], isNull);
});

testWidgets('clearing', (WidgetTester _) async {
await preferences.setString('String', kTestValues['flutter.String']);
await preferences.setBool('bool', kTestValues['flutter.bool']);
await preferences.setInt('int', kTestValues['flutter.int']);
await preferences.setDouble('double', kTestValues['flutter.double']);
await preferences.setStringList('List', kTestValues['flutter.List']);
await Future.wait(<Future<bool>>[
preferences.setValue('String', 'String', kTestValues['flutter.String']),
preferences.setValue('Bool', 'bool', kTestValues['flutter.bool']),
preferences.setValue('Int', 'int', kTestValues['flutter.int']),
preferences.setValue('Double', 'double', kTestValues['flutter.double']),
preferences.setValue('StringList', 'List', kTestValues['flutter.List'])
]);
await preferences.clear();
expect(preferences.getString('String'), null);
expect(preferences.getBool('bool'), null);
expect(preferences.getInt('int'), null);
expect(preferences.getDouble('double'), null);
expect(preferences.getStringList('List'), null);
final all = await preferences.getAll();
expect(all['String'], null);
expect(all['bool'], null);
expect(all['int'], null);
expect(all['double'], null);
expect(all['List'], null);
});
});
}
7 changes: 0 additions & 7 deletions packages/shared_preferences/shared_preferences_linux/example/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,8 @@ description: Demonstrates how to use the shared_preferences_linux plugin.
dependencies:
flutter:
sdk: flutter
shared_preferences: any
shared_preferences_linux: ^0.1.0

dependency_overrides:
shared_preferences_linux:
path: ../
# Remove this override once the endorsement is published.
shared_preferences:
path: ../../shared_preferences/

dev_dependencies:
flutter_driver:
Expand Down
23 changes: 14 additions & 9 deletions packages/shared_preferences/shared_preferences_linux/lib/shared_preferences_linux.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,43 +20,48 @@ class SharedPreferencesLinux extends SharedPreferencesStorePlatform {
static SharedPreferencesLinux instance = SharedPreferencesLinux();

/// Local copy of preferences
Map<String, Object> _cachedPreferences;
Map<String, Object>? _cachedPreferences;

/// File system used to store to disk. Exposed for testing only.
@visibleForTesting
FileSystem fs = LocalFileSystem();

/// Gets the file where the preferences are stored.
Future<File> _getLocalDataFile() async {
Future<File?> _getLocalDataFile() async {
final pathProvider = PathProviderLinux();
final directory = await pathProvider.getApplicationSupportPath();
if (directory == null) return null;
return fs.file(path.join(directory, 'shared_preferences.json'));
}

/// Gets the preferences from the stored file. Once read, the preferences are
/// maintained in memory.
Future<Map<String, Object>> _readPreferences() async {
if (_cachedPreferences != null) {
return _cachedPreferences;
return _cachedPreferences!;
}

_cachedPreferences = {};
var localDataFile = await _getLocalDataFile();
if (localDataFile.existsSync()) {
Map<String, Object> preferences = {};
final File? localDataFile = await _getLocalDataFile();
if (localDataFile != null && localDataFile.existsSync()) {
String stringMap = localDataFile.readAsStringSync();
if (stringMap.isNotEmpty) {
_cachedPreferences = json.decode(stringMap) as Map<String, Object>;
preferences = json.decode(stringMap).cast<String, Object>();
}
}

return _cachedPreferences;
_cachedPreferences = preferences;
return preferences;
}

/// Writes the cached preferences to disk. Returns [true] if the operation
/// succeeded.
Future<bool> _writePreferences(Map<String, Object> preferences) async {
try {
var localDataFile = await _getLocalDataFile();
if (localDataFile == null) {
print("Unable to determine where to write preferences.");
return false;
}
if (!localDataFile.existsSync()) {
localDataFile.createSync(recursive: true);
}
Expand Down
12 changes: 6 additions & 6 deletions packages/shared_preferences/shared_preferences_linux/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: shared_preferences_linux
description: Linux implementation of the shared_preferences plugin
version: 0.0.3+1
version: 0.0.4-nullsafety
homepage: https://github.com/flutter/plugins/tree/master/packages/shared_preferences/shared_preferences_linux

flutter:
Expand All @@ -11,17 +11,17 @@ flutter:
pluginClass: none

environment:
sdk: ">=2.1.0 <3.0.0"
sdk: ">=2.12.0-0 <3.0.0"
flutter: ">=1.12.8"

dependencies:
file: ">=5.1.0 <7.0.0"
flutter:
sdk: flutter
file: ^6.0.0-nullsafety.4
meta: ^1.0.4
path: ^1.6.4
path_provider_linux: ^0.0.1
shared_preferences_platform_interface: ^1.0.0
path: ^1.8.0-nullsafety.3
path_provider_linux: ^0.2.0-nullsafety
shared_preferences_platform_interface: ^2.0.0-nullsafety

dev_dependencies:
flutter_test:
Expand Down
6 changes: 3 additions & 3 deletions packages/shared_preferences/shared_preferences_linux/test/shared_preferences_linux_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import 'package:path/path.dart' as path;
import 'package:path_provider_linux/path_provider_linux.dart';
import 'package:shared_preferences_linux/shared_preferences_linux.dart';

MemoryFileSystem fs;

void main() {
late MemoryFileSystem fs;

setUp(() {
fs = MemoryFileSystem.test();
});
Expand All @@ -19,7 +19,7 @@ void main() {
Future<String> _getFilePath() async {
final pathProvider = PathProviderLinux();
final directory = await pathProvider.getApplicationSupportPath();
return path.join(directory, 'shared_preferences.json');
return path.join(directory!, 'shared_preferences.json');
}

_writeTestFile(String value) async {
Expand Down
4 changes: 4 additions & 0 deletions packages/shared_preferences/shared_preferences_web/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.2.0-nullsafety

* Migrate to null-safety.

## 0.1.2+8

* Update Flutter SDK constraint.
Expand Down
21 changes: 8 additions & 13 deletions packages/shared_preferences/shared_preferences_web/lib/shared_preferences_web.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import 'package:shared_preferences_platform_interface/shared_preferences_platfor
/// This class implements the `package:shared_preferences` functionality for the web.
class SharedPreferencesPlugin extends SharedPreferencesStorePlatform {
/// Registers this class as the default instance of [SharedPreferencesStorePlatform].
static void registerWith(Registrar registrar) {
static void registerWith(Registrar? registrar) {
SharedPreferencesStorePlatform.instance = SharedPreferencesPlugin();
}

Expand All @@ -31,9 +31,9 @@ class SharedPreferencesPlugin extends SharedPreferencesStorePlatform {

@override
Future<Map<String, Object>> getAll() async {
final Map<String, Object> allData = <String, Object>{};
final Map<String, Object> allData = {};
for (String key in _storedFlutterKeys) {
allData[key] = _decodeValue(html.window.localStorage[key]);
allData[key] = _decodeValue(html.window.localStorage[key]!);
}
return allData;
}
Expand All @@ -46,7 +46,7 @@ class SharedPreferencesPlugin extends SharedPreferencesStorePlatform {
}

@override
Future<bool> setValue(String valueType, String key, Object value) async {
Future<bool> setValue(String valueType, String key, Object? value) async {
_checkPrefix(key);
html.window.localStorage[key] = _encodeValue(value);
return true;
Expand All @@ -62,17 +62,12 @@ class SharedPreferencesPlugin extends SharedPreferencesStorePlatform {
}
}

List<String> get _storedFlutterKeys {
final List<String> keys = <String>[];
for (String key in html.window.localStorage.keys) {
if (key.startsWith('flutter.')) {
keys.add(key);
}
}
return keys;
Iterable<String> get _storedFlutterKeys {
return html.window.localStorage.keys
.where((key) => key.startsWith('flutter.'));
}

String _encodeValue(Object value) {
String _encodeValue(Object? value) {
return json.encode(value);
}

Expand Down
6 changes: 3 additions & 3 deletions packages/shared_preferences/shared_preferences_web/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ homepage: https://github.com/flutter/plugins/tree/master/packages/shared_prefere
# 0.1.y+z is compatible with 1.0.0, if you land a breaking change bump
# the version to 2.0.0.
# See more details: https://github.com/flutter/flutter/wiki/Package-migration-to-1.0.0
version: 0.1.2+8
version: 0.2.0-nullsafety

flutter:
plugin:
Expand All @@ -14,7 +14,7 @@ flutter:
fileName: shared_preferences_web.dart

dependencies:
shared_preferences_platform_interface: ^1.0.0
shared_preferences_platform_interface: ^2.0.0-nullsafety
flutter:
sdk: flutter
flutter_web_plugins:
Expand All @@ -27,5 +27,5 @@ dev_dependencies:
pedantic: ^1.8.0

environment:
sdk: ">=2.1.0 <3.0.0"
sdk: ">=2.12.0-0 <3.0.0"
flutter: ">=1.12.13+hotfix.4"
6 changes: 4 additions & 2 deletions packages/shared_preferences/shared_preferences_web/test/shared_preferences_web_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

@TestOn('chrome') // Uses web-only Flutter SDK

@TestOn('chrome')
import 'dart:convert' show json;
import 'dart:html' as html;

import 'package:flutter_test/flutter_test.dart';
import 'package:shared_preferences_platform_interface/method_channel_shared_preferences.dart';
import 'package:shared_preferences_platform_interface/shared_preferences_platform_interface.dart';
import 'package:shared_preferences_web/shared_preferences_web.dart';

Expand All @@ -26,6 +26,8 @@ void main() {
});

test('registers itself', () {
SharedPreferencesStorePlatform.instance =
MethodChannelSharedPreferencesStore();
expect(SharedPreferencesStorePlatform.instance,
Copy link
Contributor Author
@gaetschwartz gaetschwartz Feb 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this line because it was making the test fail and by reading SharedPreferences.registerWith(registrar)'s implementation, it always sets the instance to SharedPreferencesPlugin.

plugins/packages/shared_preferences/shared_preferences_web/lib/shared_preferences_web.dart

Lines 17 to 19 in 2e22820

static void registerWith(Registrar? registrar) {
SharedPreferencesStorePlatform.instance = SharedPreferencesPlugin();
}

Maybe I am missing something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ditman ^^^

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this again: yes, it always registers itself as a SharedPreferencesPlugin, but this check is before the registration, making sure that the later check is actually testing something meaningful.

What exactly was the error? Does this just need handling of a null instnace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, as per here :

plugins/packages/shared_preferences/shared_preferences_platform_interface/lib/shared_preferences_platform_interface.dart

Lines 38 to 39 in 2e22820

static SharedPreferencesStorePlatform _instance =
MethodChannelSharedPreferencesStore();

It is supposed to return a MethodChannelSharedPreferencesStore but the test fails on master for some reason :

00:00 +0: SharedPreferencesPlugin registers itself
00:00 +0: SharedPreferencesPlugin registers itself [E]
  Expected: not <Instance of 'SharedPreferencesPlugin'>
    Actual: <Instance of 'SharedPreferencesPlugin'>

  dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 236:49  throw_
  packages/test_api/src/frontend/expect.dart 155:31                             fail
  packages/test_api/src/frontend/expect.dart 150:3                              _expect

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Odd. How about instead of just removing that line, replacing it with manually setting the instance to a dummy value (i.e, an empty subclass created by the test) then. Otherwise in the case you are seeing, nothing is actually being tested.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is testing that the instance is null until SharedPreferencesPlugin.registerWith is called a couple of lines below.

(Maybe with null safety that part of the test is not required anymore?) We'll eventually have to revisit these tests so they run with flutter drive instead of flutter run --platform chrome. Thanks for the migration!!

isNot(isA<SharedPreferencesPlugin>()));
SharedPreferencesPlugin.registerWith(null);
Expand Down
0