Skip to content

Commit

Permalink
[file_selector] Throw for unsupported type groups (flutter#6120)
Browse files Browse the repository at this point in the history
  • Loading branch information
stuartmorgan authored and mauricioluz committed Jan 26, 2023
1 parent f08cc51 commit 69deb8f
Show file tree
Hide file tree
Showing 12 changed files with 269 additions and 72 deletions.
5 changes: 4 additions & 1 deletion packages/file_selector/file_selector_macos/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## NEXT
## 0.9.0

* **BREAKING CHANGE**: Methods that take `XTypeGroup`s now throw an
`ArgumentError` if any group is not a wildcard (all filter types null or
empty), but doesn't include any of the filter types supported by macOS.
* Ignores deprecation warnings for upcoming styleFrom button API changes.

## 0.8.2+2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,19 @@ class FileSelectorMacOS extends FileSelectorPlatform {
};
for (final XTypeGroup typeGroup in typeGroups) {
// If any group allows everything, no filtering should be done.
if (typeGroup.allowsAny) {
return null;
}
// Reject a filter that isn't an allow-any, but doesn't set any
// macOS-supported filter categories.
if ((typeGroup.extensions?.isEmpty ?? true) &&
(typeGroup.macUTIs?.isEmpty ?? true) &&
(typeGroup.mimeTypes?.isEmpty ?? true)) {
return null;
throw ArgumentError('Provided type group $typeGroup does not allow '
'all files, but does not set any of the macOS-supported filter '
'categories. At least one of "extensions", "macUTIs", or '
'"mimeTypes" must be non-empty for macOS if anything is '
'non-empty.');
}
allowedTypes[extensionKey]!.addAll(typeGroup.extensions ?? <String>[]);
allowedTypes[mimeTypeKey]!.addAll(typeGroup.mimeTypes ?? <String>[]);
Expand Down
4 changes: 2 additions & 2 deletions packages/file_selector/file_selector_macos/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: file_selector_macos
description: macOS implementation of the file_selector plugin.
repository: https://github.com/flutter/plugins/tree/main/packages/file_selector/file_selector_macos
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+file_selector%22
version: 0.8.2+2
version: 0.9.0

environment:
sdk: ">=2.12.0 <3.0.0"
Expand All @@ -18,7 +18,7 @@ flutter:

dependencies:
cross_file: ^0.3.1
file_selector_platform_interface: ^2.0.4
file_selector_platform_interface: ^2.1.0
flutter:
sdk: flutter
flutter_test:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ void main() {
],
);
});

test('passes initialDirectory correctly', () async {
await plugin.openFile(initialDirectory: '/example/directory');

Expand All @@ -77,6 +78,7 @@ void main() {
],
);
});

test('passes confirmButtonText correctly', () async {
await plugin.openFile(confirmButtonText: 'Open File');

Expand All @@ -92,7 +94,28 @@ void main() {
],
);
});

test('throws for a type group that does not support macOS', () async {
final XTypeGroup group = XTypeGroup(
label: 'images',
webWildCards: <String>['images/*'],
);

await expectLater(
plugin.openFile(acceptedTypeGroups: <XTypeGroup>[group]),
throwsArgumentError);
});

test('allows a wildcard group', () async {
final XTypeGroup group = XTypeGroup(
label: 'text',
);

await expectLater(
plugin.openFile(acceptedTypeGroups: <XTypeGroup>[group]), completes);
});
});

group('openFiles', () {
test('passes the accepted type groups correctly', () async {
final XTypeGroup group = XTypeGroup(
Expand Down Expand Up @@ -127,6 +150,7 @@ void main() {
],
);
});

test('passes initialDirectory correctly', () async {
await plugin.openFiles(initialDirectory: '/example/directory');

Expand All @@ -142,6 +166,7 @@ void main() {
],
);
});

test('passes confirmButtonText correctly', () async {
await plugin.openFiles(confirmButtonText: 'Open File');

Expand All @@ -157,6 +182,26 @@ void main() {
],
);
});

test('throws for a type group that does not support macOS', () async {
final XTypeGroup group = XTypeGroup(
label: 'images',
webWildCards: <String>['images/*'],
);

await expectLater(
plugin.openFiles(acceptedTypeGroups: <XTypeGroup>[group]),
throwsArgumentError);
});

test('allows a wildcard group', () async {
final XTypeGroup group = XTypeGroup(
label: 'text',
);

await expectLater(
plugin.openFiles(acceptedTypeGroups: <XTypeGroup>[group]), completes);
});
});

group('getSavePath', () {
Expand Down Expand Up @@ -194,6 +239,7 @@ void main() {
],
);
});

test('passes initialDirectory correctly', () async {
await plugin.getSavePath(initialDirectory: '/example/directory');

Expand All @@ -209,6 +255,7 @@ void main() {
],
);
});

test('passes confirmButtonText correctly', () async {
await plugin.getSavePath(confirmButtonText: 'Open File');

Expand All @@ -224,33 +271,56 @@ void main() {
],
);
});
group('getDirectoryPath', () {
test('passes initialDirectory correctly', () async {
await plugin.getDirectoryPath(initialDirectory: '/example/directory');

expect(
log,
<Matcher>[
isMethodCall('getDirectoryPath', arguments: <String, dynamic>{
'initialDirectory': '/example/directory',
'confirmButtonText': null,
}),
],
);
});
test('passes confirmButtonText correctly', () async {
await plugin.getDirectoryPath(confirmButtonText: 'Open File');

expect(
log,
<Matcher>[
isMethodCall('getDirectoryPath', arguments: <String, dynamic>{
'initialDirectory': null,
'confirmButtonText': 'Open File',
}),
],
);
});

test('throws for a type group that does not support macOS', () async {
final XTypeGroup group = XTypeGroup(
label: 'images',
webWildCards: <String>['images/*'],
);

await expectLater(
plugin.getSavePath(acceptedTypeGroups: <XTypeGroup>[group]),
throwsArgumentError);
});

test('allows a wildcard group', () async {
final XTypeGroup group = XTypeGroup(
label: 'text',
);

await expectLater(
plugin.getSavePath(acceptedTypeGroups: <XTypeGroup>[group]),
completes);
});
});

group('getDirectoryPath', () {
test('passes initialDirectory correctly', () async {
await plugin.getDirectoryPath(initialDirectory: '/example/directory');

expect(
log,
<Matcher>[
isMethodCall('getDirectoryPath', arguments: <String, dynamic>{
'initialDirectory': '/example/directory',
'confirmButtonText': null,
}),
],
);
});

test('passes confirmButtonText correctly', () async {
await plugin.getDirectoryPath(confirmButtonText: 'Open File');

expect(
log,
<Matcher>[
isMethodCall('getDirectoryPath', arguments: <String, dynamic>{
'initialDirectory': null,
'confirmButtonText': 'Open File',
}),
],
);
});
});

Expand Down
6 changes: 6 additions & 0 deletions packages/file_selector/file_selector_web/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 0.9.0

* **BREAKING CHANGE**: Methods that take `XTypeGroup`s now throw an
`ArgumentError` if any group is not a wildcard (all filter types null or
empty), but doesn't include any of the filter types supported by web.

## 0.8.1+5

* Minor fixes for new analysis options.
Expand Down
24 changes: 16 additions & 8 deletions packages/file_selector/file_selector_web/lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ String acceptedTypesToString(List<XTypeGroup>? acceptedTypes) {
}
final List<String> allTypes = <String>[];
for (final XTypeGroup group in acceptedTypes) {
_assertTypeGroupIsValid(group);
// If any group allows everything, no filtering should be done.
if (group.allowsAny) {
return '';
}
_validateTypeGroup(group);
if (group.extensions != null) {
allTypes.addAll(group.extensions!.map(_normalizeExtension));
}
Expand All @@ -25,13 +29,17 @@ String acceptedTypesToString(List<XTypeGroup>? acceptedTypes) {
return allTypes.join(',');
}

/// Make sure that at least one of its fields is populated.
void _assertTypeGroupIsValid(XTypeGroup group) {
assert(
!((group.extensions == null || group.extensions!.isEmpty) &&
(group.mimeTypes == null || group.mimeTypes!.isEmpty) &&
(group.webWildCards == null || group.webWildCards!.isEmpty)),
'At least one of extensions / mimeTypes / webWildCards is required for web.');
/// Make sure that at least one of the supported fields is populated.
void _validateTypeGroup(XTypeGroup group) {
if ((group.extensions?.isEmpty ?? true) &&
(group.mimeTypes?.isEmpty ?? true) &&
(group.webWildCards?.isEmpty ?? true)) {
throw ArgumentError('Provided type group $group does not allow '
'all files, but does not set any of the web-supported filter '
'categories. At least one of "extensions", "mimeTypes", or '
'"webWildCards" must be non-empty for web if anything is '
'non-empty.');
}
}

/// Append a dot at the beggining if it is not there png -> .png
Expand Down
4 changes: 2 additions & 2 deletions packages/file_selector/file_selector_web/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: file_selector_web
description: Web platform implementation of file_selector
repository: https://github.com/flutter/plugins/tree/main/packages/file_selector/file_selector_web
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+file_selector%22
version: 0.8.1+5
version: 0.9.0

environment:
sdk: ">=2.12.0 <3.0.0"
Expand All @@ -17,7 +17,7 @@ flutter:
fileName: file_selector_web.dart

dependencies:
file_selector_platform_interface: ^2.0.0
file_selector_platform_interface: ^2.1.0
flutter:
sdk: flutter
flutter_web_plugins:
Expand Down
7 changes: 7 additions & 0 deletions packages/file_selector/file_selector_web/test/utils_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ void main() {
final String accepts = acceptedTypesToString(acceptedTypes);
expect(accepts, 'image/*,audio/*,video/*');
});

test('throws for a type group that does not support web', () {
final List<XTypeGroup> acceptedTypes = <XTypeGroup>[
XTypeGroup(label: 'text', macUTIs: <String>['public.text']),
];
expect(() => acceptedTypesToString(acceptedTypes), throwsArgumentError);
});
});
});
}
5 changes: 4 additions & 1 deletion packages/file_selector/file_selector_windows/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## NEXT
## 0.9.0

* **BREAKING CHANGE**: Methods that take `XTypeGroup`s now throw an
`ArgumentError` if any group is not a wildcard (all filter types null or
empty), but doesn't include any of the filter types supported by Windows.
* Ignores deprecation warnings for upcoming styleFrom button API changes.

## 0.8.2+2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class FileSelectorWindows extends FileSelectorPlatform {
String? initialDirectory,
String? confirmButtonText,
}) async {
_validateTypeGroups(acceptedTypeGroups);
final List<String>? path = await _channel.invokeListMethod<String>(
'openFile',
<String, dynamic>{
Expand All @@ -46,6 +47,7 @@ class FileSelectorWindows extends FileSelectorPlatform {
String? initialDirectory,
String? confirmButtonText,
}) async {
_validateTypeGroups(acceptedTypeGroups);
final List<String>? pathList = await _channel.invokeListMethod<String>(
'openFile',
<String, dynamic>{
Expand All @@ -67,6 +69,7 @@ class FileSelectorWindows extends FileSelectorPlatform {
String? suggestedName,
String? confirmButtonText,
}) async {
_validateTypeGroups(acceptedTypeGroups);
return _channel.invokeMethod<String>(
'getSavePath',
<String, dynamic>{
Expand All @@ -93,4 +96,20 @@ class FileSelectorWindows extends FileSelectorPlatform {
},
);
}

/// Throws an [ArgumentError] if any of the provided type groups are not valid
/// for Windows.
void _validateTypeGroups(List<XTypeGroup>? groups) {
if (groups == null) {
return;
}
for (final XTypeGroup group in groups) {
if (!group.allowsAny && (group.extensions?.isEmpty ?? true)) {
throw ArgumentError('Provided type group $group does not allow '
'all files, but does not set any of the Windows-supported filter '
'categories. "extensions" must be non-empty for Windows if '
'anything is non-empty.');
}
}
}
}
4 changes: 2 additions & 2 deletions packages/file_selector/file_selector_windows/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: file_selector_windows
description: Windows implementation of the file_selector plugin.
repository: https://github.com/flutter/plugins/tree/main/packages/file_selector/file_selector_windows
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+file_selector%22
version: 0.8.2+2
version: 0.9.0

environment:
sdk: ">=2.12.0 <3.0.0"
Expand All @@ -18,7 +18,7 @@ flutter:

dependencies:
cross_file: ^0.3.1
file_selector_platform_interface: ^2.0.4
file_selector_platform_interface: ^2.1.0
flutter:
sdk: flutter
flutter_test:
Expand Down
Loading

0 comments on commit 69deb8f

Please sign in to comment.