[image_picker] Fix exception when canceling pickMultipleMedia on iOS (#4761)

Makes the Pigeon interface for the iOS implementation consistent in returning non-nullable lists, in keeping with our strong preference in this repo for collections being non-nullable whenever reasonable (to avoid the issue of having two ways of expressing the same thing), and updates the native side never to return a `nil` list.

Fixes https://github.com/flutter/flutter/issues/133111
This commit is contained in:
stuartmorgan
2023-08-23 10:56:22 -07:00
committed by GitHub
parent c0136f9c32
commit 32460c7da3
9 changed files with 49 additions and 30 deletions

View File

@ -1,5 +1,6 @@
## NEXT ## 0.8.8+1
* Fixes exception when canceling pickMultipleMedia.
* Updates minimum supported SDK version to Flutter 3.7/Dart 2.19. * Updates minimum supported SDK version to Flutter 3.7/Dart 2.19.
## 0.8.8 ## 0.8.8

View File

@ -360,6 +360,20 @@
[self waitForExpectationsWithTimeout:30 handler:nil]; [self waitForExpectationsWithTimeout:30 handler:nil];
} }
- (void)testPluginMediaPathConvertsNilToEmptyList {
FLTImagePickerPlugin *plugin = [[FLTImagePickerPlugin alloc] init];
XCTestExpectation *resultExpectation = [self expectationWithDescription:@"result"];
plugin.callContext = [[FLTImagePickerMethodCallContext alloc]
initWithResult:^(NSArray<NSString *> *_Nullable result, FlutterError *_Nullable error) {
XCTAssertEqualObjects(result, @[]);
[resultExpectation fulfill];
}];
[plugin sendCallResultWithSavedPathList:nil];
[self waitForExpectationsWithTimeout:30 handler:nil];
}
- (void)testPluginMediaPathHasItem { - (void)testPluginMediaPathHasItem {
FLTImagePickerPlugin *plugin = [[FLTImagePickerPlugin alloc] init]; FLTImagePickerPlugin *plugin = [[FLTImagePickerPlugin alloc] init];
NSArray *pathList = @[ @"test" ]; NSArray *pathList = @[ @"test" ];

View File

@ -173,7 +173,7 @@ typedef NS_ENUM(NSInteger, ImagePickerClassType) { UIImagePickerClassType, PHPic
[self cancelInProgressCall]; [self cancelInProgressCall];
FLTImagePickerMethodCallContext *context = [[FLTImagePickerMethodCallContext alloc] FLTImagePickerMethodCallContext *context = [[FLTImagePickerMethodCallContext alloc]
initWithResult:^void(NSArray<NSString *> *paths, FlutterError *error) { initWithResult:^void(NSArray<NSString *> *paths, FlutterError *error) {
if (paths && paths.count != 1) { if (paths.count > 1) {
completion(nil, [FlutterError errorWithCode:@"invalid_result" completion(nil, [FlutterError errorWithCode:@"invalid_result"
message:@"Incorrect number of return paths provided" message:@"Incorrect number of return paths provided"
details:nil]); details:nil]);
@ -246,7 +246,7 @@ typedef NS_ENUM(NSInteger, ImagePickerClassType) { UIImagePickerClassType, PHPic
(nonnull void (^)(NSString *_Nullable, FlutterError *_Nullable))completion { (nonnull void (^)(NSString *_Nullable, FlutterError *_Nullable))completion {
FLTImagePickerMethodCallContext *context = [[FLTImagePickerMethodCallContext alloc] FLTImagePickerMethodCallContext *context = [[FLTImagePickerMethodCallContext alloc]
initWithResult:^void(NSArray<NSString *> *paths, FlutterError *error) { initWithResult:^void(NSArray<NSString *> *paths, FlutterError *error) {
if (paths && paths.count != 1) { if (paths.count > 1) {
completion(nil, [FlutterError errorWithCode:@"invalid_result" completion(nil, [FlutterError errorWithCode:@"invalid_result"
message:@"Incorrect number of return paths provided" message:@"Incorrect number of return paths provided"
details:nil]); details:nil]);
@ -687,7 +687,7 @@ typedef NS_ENUM(NSInteger, ImagePickerClassType) { UIImagePickerClassType, PHPic
message:@"pathList's items should not be null" message:@"pathList's items should not be null"
details:nil]); details:nil]);
} else { } else {
self.callContext.result(pathList, nil); self.callContext.result(pathList ?: [NSArray array], nil);
} }
self.callContext = nil; self.callContext = nil;
} }

View File

@ -88,7 +88,7 @@ class ImagePickerIOS extends ImagePickerPlatform {
double? maxHeight, double? maxHeight,
int? imageQuality, int? imageQuality,
}) async { }) async {
final List<dynamic>? paths = await _pickMultiImageAsPath( final List<dynamic> paths = await _pickMultiImageAsPath(
options: MultiImagePickerOptions( options: MultiImagePickerOptions(
imageOptions: ImageOptions( imageOptions: ImageOptions(
maxWidth: maxWidth, maxWidth: maxWidth,
@ -97,7 +97,9 @@ class ImagePickerIOS extends ImagePickerPlatform {
), ),
), ),
); );
if (paths == null) { // Convert an empty list to a null return since that was the legacy behavior
// of this method.
if (paths.isEmpty) {
return null; return null;
} }
@ -108,15 +110,11 @@ class ImagePickerIOS extends ImagePickerPlatform {
Future<List<XFile>> getMultiImageWithOptions({ Future<List<XFile>> getMultiImageWithOptions({
MultiImagePickerOptions options = const MultiImagePickerOptions(), MultiImagePickerOptions options = const MultiImagePickerOptions(),
}) async { }) async {
final List<String>? paths = await _pickMultiImageAsPath(options: options); final List<String> paths = await _pickMultiImageAsPath(options: options);
if (paths == null) {
return <XFile>[];
}
return paths.map((String path) => XFile(path)).toList(); return paths.map((String path) => XFile(path)).toList();
} }
Future<List<String>?> _pickMultiImageAsPath({ Future<List<String>> _pickMultiImageAsPath({
MultiImagePickerOptions options = const MultiImagePickerOptions(), MultiImagePickerOptions options = const MultiImagePickerOptions(),
}) async { }) async {
final int? imageQuality = options.imageOptions.imageQuality; final int? imageQuality = options.imageOptions.imageQuality;
@ -141,7 +139,7 @@ class ImagePickerIOS extends ImagePickerPlatform {
MaxSize(width: maxWidth, height: maxHeight), MaxSize(width: maxWidth, height: maxHeight),
imageQuality, imageQuality,
options.imageOptions.requestFullMetadata)) options.imageOptions.requestFullMetadata))
?.cast<String>(); .cast<String>();
} }
Future<String?> _pickImageAsPath({ Future<String?> _pickImageAsPath({
@ -272,7 +270,7 @@ class ImagePickerIOS extends ImagePickerPlatform {
double? maxHeight, double? maxHeight,
int? imageQuality, int? imageQuality,
}) async { }) async {
final List<String>? paths = await _pickMultiImageAsPath( final List<String> paths = await _pickMultiImageAsPath(
options: MultiImagePickerOptions( options: MultiImagePickerOptions(
imageOptions: ImageOptions( imageOptions: ImageOptions(
maxWidth: maxWidth, maxWidth: maxWidth,
@ -281,7 +279,9 @@ class ImagePickerIOS extends ImagePickerPlatform {
), ),
), ),
); );
if (paths == null) { // Convert an empty list to a null return since that was the legacy behavior
// of this method.
if (paths.isEmpty) {
return null; return null;
} }

View File

@ -179,7 +179,7 @@ class ImagePickerApi {
} }
} }
Future<List<String?>?> pickMultiImage(MaxSize arg_maxSize, Future<List<String?>> pickMultiImage(MaxSize arg_maxSize,
int? arg_imageQuality, bool arg_requestFullMetadata) async { int? arg_imageQuality, bool arg_requestFullMetadata) async {
final BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>( final BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
'dev.flutter.pigeon.ImagePickerApi.pickMultiImage', codec, 'dev.flutter.pigeon.ImagePickerApi.pickMultiImage', codec,
@ -198,8 +198,13 @@ class ImagePickerApi {
message: replyList[1] as String?, message: replyList[1] as String?,
details: replyList[2], details: replyList[2],
); );
} else if (replyList[0] == null) {
throw PlatformException(
code: 'null-error',
message: 'Host platform returned null value for non-null return value.',
);
} else { } else {
return (replyList[0] as List<Object?>?)?.cast<String?>(); return (replyList[0] as List<Object?>?)!.cast<String?>();
} }
} }

View File

@ -54,7 +54,7 @@ abstract class ImagePickerApi {
int? imageQuality, bool requestFullMetadata); int? imageQuality, bool requestFullMetadata);
@async @async
@ObjCSelector('pickMultiImageWithMaxSize:quality:fullMetadata:') @ObjCSelector('pickMultiImageWithMaxSize:quality:fullMetadata:')
List<String>? pickMultiImage( List<String?> pickMultiImage(
MaxSize maxSize, int? imageQuality, bool requestFullMetadata); MaxSize maxSize, int? imageQuality, bool requestFullMetadata);
@async @async
@ObjCSelector('pickVideoWithSource:maxDuration:') @ObjCSelector('pickVideoWithSource:maxDuration:')

View File

@ -2,7 +2,7 @@ name: image_picker_ios
description: iOS implementation of the image_picker plugin. description: iOS implementation of the image_picker plugin.
repository: https://github.com/flutter/packages/tree/main/packages/image_picker/image_picker_ios repository: https://github.com/flutter/packages/tree/main/packages/image_picker/image_picker_ios
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+image_picker%22 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+image_picker%22
version: 0.8.8 version: 0.8.8+1
environment: environment:
sdk: ">=2.19.0 <4.0.0" sdk: ">=2.19.0 <4.0.0"

View File

@ -57,7 +57,7 @@ class _ApiLogger implements TestHostImagePickerApi {
} }
@override @override
Future<List<String?>?> pickMultiImage( Future<List<String?>> pickMultiImage(
MaxSize maxSize, MaxSize maxSize,
int? imageQuality, int? imageQuality,
bool requestFullMetadata, bool requestFullMetadata,
@ -68,7 +68,7 @@ class _ApiLogger implements TestHostImagePickerApi {
'imageQuality': imageQuality, 'imageQuality': imageQuality,
'requestFullMetadata': requestFullMetadata, 'requestFullMetadata': requestFullMetadata,
})); }));
return returnValue as List<String?>?; return returnValue as List<String?>;
} }
@override @override
@ -441,8 +441,8 @@ void main() {
); );
}); });
test('handles a null image path response gracefully', () async { test('returns null for an empty list', () async {
log.returnValue = null; log.returnValue = <String>[];
expect(await picker.pickMultiImage(), isNull); expect(await picker.pickMultiImage(), isNull);
}); });
@ -883,11 +883,10 @@ void main() {
); );
}); });
test('handles a null image path response gracefully', () async { test('returns null for an empty list', () async {
log.returnValue = null; log.returnValue = <String>[];
expect(await picker.getMultiImage(), isNull); expect(await picker.getMultiImage(), isNull);
expect(await picker.getMultiImage(), isNull);
}); });
}); });
@ -1643,8 +1642,8 @@ void main() {
); );
}); });
test('handles a null image path response gracefully', () async { test('handles an empty response', () async {
log.returnValue = null; log.returnValue = <String>[];
expect(await picker.getMultiImageWithOptions(), isEmpty); expect(await picker.getMultiImageWithOptions(), isEmpty);
}); });

View File

@ -54,7 +54,7 @@ abstract class TestHostImagePickerApi {
Future<String?> pickImage(SourceSpecification source, MaxSize maxSize, Future<String?> pickImage(SourceSpecification source, MaxSize maxSize,
int? imageQuality, bool requestFullMetadata); int? imageQuality, bool requestFullMetadata);
Future<List<String?>?> pickMultiImage( Future<List<String?>> pickMultiImage(
MaxSize maxSize, int? imageQuality, bool requestFullMetadata); MaxSize maxSize, int? imageQuality, bool requestFullMetadata);
Future<String?> pickVideo( Future<String?> pickVideo(
@ -117,7 +117,7 @@ abstract class TestHostImagePickerApi {
final bool? arg_requestFullMetadata = (args[2] as bool?); final bool? arg_requestFullMetadata = (args[2] as bool?);
assert(arg_requestFullMetadata != null, assert(arg_requestFullMetadata != null,
'Argument for dev.flutter.pigeon.ImagePickerApi.pickMultiImage was null, expected non-null bool.'); 'Argument for dev.flutter.pigeon.ImagePickerApi.pickMultiImage was null, expected non-null bool.');
final List<String?>? output = await api.pickMultiImage( final List<String?> output = await api.pickMultiImage(
arg_maxSize!, arg_imageQuality, arg_requestFullMetadata!); arg_maxSize!, arg_imageQuality, arg_requestFullMetadata!);
return <Object?>[output]; return <Object?>[output];
}); });