Skip to content
This repository has been 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
Feb 8, 2021
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
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
12 changes: 6 additions & 6 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,36 +20,36 @@ 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();
FileSystem? fs = LocalFileSystem();

/// Gets the file where the preferences are stored.
Future<File> _getLocalDataFile() async {
final pathProvider = PathProviderLinux();
final directory = await pathProvider.getApplicationSupportPath();
return fs.file(path.join(directory, 'shared_preferences.json'));
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()) {
String stringMap = localDataFile.readAsStringSync();
if (stringMap.isNotEmpty) {
_cachedPreferences = json.decode(stringMap) as Map<String, Object>;
_cachedPreferences = json.decode(stringMap) as Map<String, Object>?;
}
}

return _cachedPreferences;
return _cachedPreferences!;
gaetschwartz marked this conversation as resolved.
Show resolved Hide resolved
}

/// Writes the cached preferences to disk. Returns [true] if the operation
Expand Down
11 changes: 5 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,16 @@ flutter:
pluginClass: none

environment:
sdk: ">=2.1.0 <3.0.0"
sdk: ">=2.12.0-259.8.beta <3.0.0"
gaetschwartz marked this conversation as resolved.
Show resolved Hide resolved
flutter: ">=1.12.8"

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

dev_dependencies:
flutter_test:
Expand Down
8 changes: 4 additions & 4 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,7 +7,7 @@ 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;
MemoryFileSystem? fs;

void main() {
setUp(() {
Expand All @@ -19,17 +19,17 @@ 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 {
fs.file(await _getFilePath())
fs!.file(await _getFilePath())
..createSync(recursive: true)
..writeAsStringSync(value);
}

Future<String> _readTestFile() async {
return fs.file(await _getFilePath()).readAsStringSync();
return fs!.file(await _getFilePath()).readAsStringSync();
}

SharedPreferencesLinux _getPreferences() {
Expand Down
20 changes: 7 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]!);
stuartmorgan marked this conversation as resolved.
Show resolved Hide resolved
}
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,11 @@ 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-259.8.beta <3.0.0"
gaetschwartz marked this conversation as resolved.
Show resolved Hide resolved
flutter: ">=1.12.13+hotfix.4"
8 changes: 2 additions & 6 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,8 +2,7 @@
// 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;

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

test('registers itself', () {
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);
expect(SharedPreferencesStorePlatform.instance,
isA<SharedPreferencesPlugin>());
expect(SharedPreferencesStorePlatform.instance, isA<SharedPreferencesPlugin>());
});

test('getAll', () async {
Expand Down
22 changes: 11 additions & 11 deletions packages/shared_preferences/shared_preferences_windows/lib/shared_preferences_windows.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@

import 'dart:async';
import 'dart:convert' show json;

import 'package:file/file.dart';
import 'package:file/local.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart' as path;
import 'package:shared_preferences_platform_interface/shared_preferences_platform_interface.dart';
import 'package:path_provider_windows/path_provider_windows.dart';
import 'package:shared_preferences_platform_interface/shared_preferences_platform_interface.dart';

/// The Windows implementation of [SharedPreferencesStorePlatform].
///
Expand All @@ -20,26 +21,25 @@ class SharedPreferencesWindows extends SharedPreferencesStorePlatform {

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

/// The path_provider_windows instance used to find the support directory.
@visibleForTesting
PathProviderWindows pathProvider = PathProviderWindows();
PathProviderWindows? pathProvider = PathProviderWindows();

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

/// Cached file for storing preferences.
File _localDataFilePath;
File? _localDataFilePath;

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

/// Gets the preferences from the stored file. Once read, the preferences are
Expand All @@ -51,11 +51,11 @@ class SharedPreferencesWindows extends SharedPreferencesStorePlatform {
if (localDataFile.existsSync()) {
String stringMap = localDataFile.readAsStringSync();
if (stringMap.isNotEmpty) {
_cachedPreferences = json.decode(stringMap) as Map<String, Object>;
_cachedPreferences = (json.decode(stringMap) as Map).cast<String, Object>();
}
}
}
return _cachedPreferences;
return _cachedPreferences!;
}

/// Writes the cached preferences to disk. Returns [true] if the operation
Expand Down
12 changes: 6 additions & 6 deletions packages/shared_preferences/shared_preferences_windows/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: shared_preferences_windows
description: Windows implementation of shared_preferences
homepage: https://github.com/flutter/plugins/tree/master/packages/shared_preferences/shared_preferences_windows
version: 0.0.2+3
version: 0.0.3-nullsafety

flutter:
plugin:
Expand All @@ -11,18 +11,18 @@ flutter:
pluginClass: none

environment:
sdk: ">=2.1.0 <3.0.0"
sdk: ">=2.12.0-259.8.beta <3.0.0"
flutter: ">=1.12.8"

dependencies:
shared_preferences_platform_interface: ^1.0.0
shared_preferences_platform_interface: ^2.0.0-nullsafety
flutter:
sdk: flutter
file: ">=5.1.0 <7.0.0"
file: ">=6.0.0-nullsafety.4 <7.0.0"
meta: ^1.1.7
path: ^1.6.4
path_provider_platform_interface: ^1.0.3
path_provider_windows: ^0.0.2
path_provider_platform_interface: ^2.0.0-nullsafety
path_provider_windows: ^0.1.0-nullsafety.2

dev_dependencies:
flutter_test:
Expand Down
27 changes: 13 additions & 14 deletions ...s/shared_preferences/shared_preferences_windows/test/shared_preferences_windows_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import 'package:path_provider_windows/path_provider_windows.dart';
import 'package:shared_preferences_windows/shared_preferences_windows.dart';

void main() {
MemoryFileSystem fileSystem;
PathProviderWindows pathProvider;
MemoryFileSystem? fileSystem;
PathProviderWindows? pathProvider;

setUp(() {
fileSystem = MemoryFileSystem.test();
Expand All @@ -21,18 +21,18 @@ void main() {
tearDown(() {});

Future<String> _getFilePath() async {
final directory = await pathProvider.getApplicationSupportPath();
return path.join(directory, 'shared_preferences.json');
final directory = await pathProvider!.getApplicationSupportPath();
return path.join(directory!, 'shared_preferences.json');
}

_writeTestFile(String value) async {
fileSystem.file(await _getFilePath())
fileSystem!.file(await _getFilePath())
..createSync(recursive: true)
..writeAsStringSync(value);
}

Future<String> _readTestFile() async {
return fileSystem.file(await _getFilePath()).readAsStringSync();
return fileSystem!.file(await _getFilePath()).readAsStringSync();
}

SharedPreferencesWindows _getPreferences() {
Expand Down Expand Up @@ -85,25 +85,24 @@ void main() {
///
/// Note that this should only be used with an in-memory filesystem, as the
/// path it returns is a root path that does not actually exist on Windows.
class FakePathProviderWindows extends PathProviderPlatform
implements PathProviderWindows {
VersionInfoQuerier versionInfoQuerier;
class FakePathProviderWindows extends PathProviderPlatform implements PathProviderWindows {
late VersionInfoQuerier versionInfoQuerier;

@override
Future<String> getApplicationSupportPath() async => r'C:\appsupport';

@override
Future<String> getTemporaryPath() async => null;
Future<String?> getTemporaryPath() async => null;

@override
Future<String> getLibraryPath() async => null;
Future<String?> getLibraryPath() async => null;

@override
Future<String> getApplicationDocumentsPath() async => null;
Future<String?> getApplicationDocumentsPath() async => null;

@override
Future<String> getDownloadsPath() async => null;
Future<String?> getDownloadsPath() async => null;

@override
Future<String> getPath(String folderID) async => null;
Future<String> getPath(String folderID) async => '';
}