-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[path_provider] Switch Android to an internal method channel #4617
Changes from 5 commits
2128e92
ada9fa5
701ad91
18edec9
548a549
9724575
d83d022
3aab432
e4e868a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'package:flutter/services.dart'; | ||
| import 'package:meta/meta.dart'; | ||
| import 'package:path_provider_platform_interface/path_provider_platform_interface.dart'; | ||
|
|
||
| /// The Android implementation of [PathProviderPlatform]. | ||
| class PathProviderAndroid extends PathProviderPlatform { | ||
| /// The method channel used to interact with the native platform. | ||
| @visibleForTesting | ||
| MethodChannel methodChannel = | ||
| const MethodChannel('plugins.flutter.io/path_provider_android'); | ||
|
|
||
| /// Registers this class as the default instance of [PathProviderPlatform]. | ||
| static void registerWith() { | ||
| PathProviderPlatform.instance = PathProviderAndroid(); | ||
| } | ||
|
|
||
| @override | ||
| Future<String?> getTemporaryPath() { | ||
| return methodChannel.invokeMethod<String>('getTemporaryDirectory'); | ||
| } | ||
|
|
||
| @override | ||
| Future<String?> getApplicationSupportPath() { | ||
| return methodChannel.invokeMethod<String>('getApplicationSupportDirectory'); | ||
| } | ||
|
|
||
| @override | ||
| Future<String?> getLibraryPath() { | ||
| throw UnsupportedError('getLibraryPath is not supported on Android'); | ||
| } | ||
|
|
||
| @override | ||
| Future<String?> getApplicationDocumentsPath() { | ||
| return methodChannel | ||
| .invokeMethod<String>('getApplicationDocumentsDirectory'); | ||
| } | ||
|
|
||
| @override | ||
| Future<String?> getExternalStoragePath() { | ||
| return methodChannel.invokeMethod<String>('getStorageDirectory'); | ||
| } | ||
|
|
||
| @override | ||
| Future<List<String>?> getExternalCachePaths() { | ||
| return methodChannel | ||
| .invokeListMethod<String>('getExternalCacheDirectories'); | ||
| } | ||
|
|
||
| @override | ||
| Future<List<String>?> getExternalStoragePaths({ | ||
| StorageDirectory? type, | ||
| }) async { | ||
| return methodChannel.invokeListMethod<String>( | ||
| 'getExternalStorageDirectories', | ||
| <String, dynamic>{'type': type?.index}, | ||
| ); | ||
| } | ||
|
|
||
| @override | ||
| Future<String?> getDownloadsPath() { | ||
| throw UnsupportedError('getDownloadsPath is not supported on Android'); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ name: path_provider_android | |
| description: Android implementation of the path_provider plugin. | ||
| repository: https://github.com/flutter/plugins/tree/master/packages/path_provider/path_provider_android | ||
| issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+path_provider%22 | ||
| version: 2.0.9 | ||
| version: 2.0.10 | ||
|
|
||
| environment: | ||
| sdk: ">=2.14.0 <3.0.0" | ||
|
|
@@ -15,11 +15,13 @@ flutter: | |
| android: | ||
| package: io.flutter.plugins.pathprovider | ||
| pluginClass: PathProviderPlugin | ||
| dartPluginClass: PathProviderAndroid | ||
|
|
||
| dependencies: | ||
| flutter: | ||
| sdk: flutter | ||
| path_provider_platform_interface: ^2.0.0 | ||
| meta: ^1.3.0 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TIL! We import
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Filed flutter/flutter#95658 |
||
| path_provider_platform_interface: ^2.0.1 | ||
|
|
||
| dev_dependencies: | ||
| flutter_driver: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'package:flutter/services.dart'; | ||
| import 'package:flutter_test/flutter_test.dart'; | ||
| import 'package:path_provider_android/path_provider_android.dart'; | ||
| import 'package:path_provider_platform_interface/src/enums.dart'; | ||
|
guille-ideotec marked this conversation as resolved.
Outdated
|
||
|
|
||
| void main() { | ||
| TestWidgetsFlutterBinding.ensureInitialized(); | ||
| const String kTemporaryPath = 'temporaryPath'; | ||
| const String kApplicationSupportPath = 'applicationSupportPath'; | ||
| const String kLibraryPath = 'libraryPath'; | ||
| const String kApplicationDocumentsPath = 'applicationDocumentsPath'; | ||
| const String kExternalCachePaths = 'externalCachePaths'; | ||
| const String kExternalStoragePaths = 'externalStoragePaths'; | ||
| const String kDownloadsPath = 'downloadsPath'; | ||
|
|
||
| group('PathProviderAndroid', () { | ||
| late PathProviderAndroid pathProvider; | ||
| final List<MethodCall> log = <MethodCall>[]; | ||
|
|
||
| setUp(() async { | ||
| pathProvider = PathProviderAndroid(); | ||
| TestDefaultBinaryMessengerBinding.instance!.defaultBinaryMessenger | ||
| .setMockMethodCallHandler(pathProvider.methodChannel, | ||
| (MethodCall methodCall) async { | ||
| log.add(methodCall); | ||
| switch (methodCall.method) { | ||
| case 'getTemporaryDirectory': | ||
| return kTemporaryPath; | ||
| case 'getApplicationSupportDirectory': | ||
| return kApplicationSupportPath; | ||
| case 'getLibraryDirectory': | ||
| return kLibraryPath; | ||
| case 'getApplicationDocumentsDirectory': | ||
| return kApplicationDocumentsPath; | ||
| case 'getExternalStorageDirectories': | ||
| return <String>[kExternalStoragePaths]; | ||
| case 'getExternalCacheDirectories': | ||
| return <String>[kExternalCachePaths]; | ||
| case 'getDownloadsDirectory': | ||
| return kDownloadsPath; | ||
| default: | ||
| return null; | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| tearDown(() { | ||
| log.clear(); | ||
| }); | ||
|
|
||
| test('getTemporaryPath', () async { | ||
| final String? path = await pathProvider.getTemporaryPath(); | ||
| expect( | ||
| log, | ||
| <Matcher>[isMethodCall('getTemporaryDirectory', arguments: null)], | ||
| ); | ||
| expect(path, kTemporaryPath); | ||
| }); | ||
|
|
||
| test('getApplicationSupportPath', () async { | ||
| final String? path = await pathProvider.getApplicationSupportPath(); | ||
| expect( | ||
| log, | ||
| <Matcher>[ | ||
| isMethodCall('getApplicationSupportDirectory', arguments: null) | ||
| ], | ||
| ); | ||
| expect(path, kApplicationSupportPath); | ||
| }); | ||
|
|
||
| test('getApplicationDocumentsPath', () async { | ||
| final String? path = await pathProvider.getApplicationDocumentsPath(); | ||
| expect( | ||
| log, | ||
| <Matcher>[ | ||
| isMethodCall('getApplicationDocumentsDirectory', arguments: null) | ||
| ], | ||
| ); | ||
| expect(path, kApplicationDocumentsPath); | ||
| }); | ||
|
|
||
| for (final StorageDirectory? type in <StorageDirectory?>[ | ||
| null, | ||
| ...StorageDirectory.values | ||
| ]) { | ||
| test('getExternalStoragePaths (type: $type) android succeeds', () async { | ||
| final List<String>? result = | ||
| await pathProvider.getExternalStoragePaths(type: type); | ||
| expect( | ||
| log, | ||
| <Matcher>[ | ||
| isMethodCall( | ||
| 'getExternalStorageDirectories', | ||
| arguments: <String, dynamic>{'type': type?.index}, | ||
| ) | ||
| ], | ||
| ); | ||
|
|
||
| expect(result!.length, 1); | ||
| expect(result.first, kExternalStoragePaths); | ||
| }); | ||
| } // end of for-loop | ||
|
|
||
| test('getExternalCachePaths succeeds', () async { | ||
| final List<String>? result = await pathProvider.getExternalCachePaths(); | ||
| expect( | ||
| log, | ||
| <Matcher>[isMethodCall('getExternalCacheDirectories', arguments: null)], | ||
| ); | ||
| expect(result!.length, 1); | ||
| expect(result.first, kExternalCachePaths); | ||
| }); | ||
|
|
||
| test('getExternalStoragePaths', () async { | ||
| final List<String>? result = await pathProvider.getExternalStoragePaths(); | ||
| expect( | ||
| log, | ||
| <Matcher>[ | ||
| isMethodCall('getExternalStorageDirectories', arguments: null) | ||
| ], | ||
| ); | ||
| expect(result, kExternalStoragePaths); | ||
| }); | ||
|
|
||
| test('getLibraryPath fails', () async { | ||
| try { | ||
| await pathProvider.getLibraryPath(); | ||
| fail('should throw UnsupportedError'); | ||
| } catch (e) { | ||
| expect(e, isUnsupportedError); | ||
| } | ||
| }); | ||
|
|
||
| test('getDownloadsPath fails', () async { | ||
| try { | ||
| await pathProvider.getDownloadsPath(); | ||
| fail('should throw UnsupportedError'); | ||
| } catch (e) { | ||
| expect(e, isUnsupportedError); | ||
| } | ||
| }); | ||
| }); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized that I forgot we needed to bump the Flutter SDK requirement to 2.8 as part of this since it uses Dart plugin registration :(
I don't think it's really fixable unfortunately, unless we:
because if we just bump the SDK requirement this incorrect version will be the last one 2.5 users will resolve to. We can wait and see if there's non-trivial fallout from this first. (If it's only a small number of people, we can tell them to temporarily pin a path_provider_android dependency.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're getting a fair number of reports of this already (flutter/flutter#95706), so we'll need to fix this.
Instead of a full revert, I think we could temporarily change the channel name back since nothing about the channel has changed yet, then change it back with an SDK bump.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a new PR reverting the channel name. I would need to know if it requires a version bump (with corresponding CHANGELOG.md line) and I guess I also need an exception for new tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#4636