Skip to content

Commit

Permalink
feat: improve error handling, avoid hardcoding string error codes, us…
Browse files Browse the repository at this point in the history 
…e pigeon for type-saftey
  • Loading branch information
@EchoEllet
EchoEllet committed Jan 4, 2025
1 parent 53c3063 commit 2a101a9
Show file tree
Hide file tree
Showing 12 changed files with 656 additions and 201 deletions.
16 changes: 0 additions & 16 deletions packages/image_picker/image_picker_macos/example/integration_test/image_picker_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
// found in the LICENSE file.

import 'package:example/main.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:image_picker_macos/image_picker_macos.dart';
import 'package:image_picker_macos/src/messages.g.dart';
import 'package:image_picker_platform_interface/image_picker_platform_interface.dart';
import 'package:integration_test/integration_test.dart';

Expand Down Expand Up @@ -44,19 +42,5 @@ void main() {
reason: 'Pressing the toggle button should update it correctly');
},
);
testWidgets(
'multi-video selection is not implemented',
(WidgetTester tester) async {
final ImagePickerApi hostApi = ImagePickerApi();
await expectLater(
hostApi.pickVideos(GeneralOptions(limit: 2)),
throwsA(predicate<PlatformException>(
(PlatformException e) =>
e.code == 'UNIMPLEMENTED' &&
e.message == 'Multi-video selection is not implemented',
)),
);
},
);
});
}
2 changes: 1 addition & 1 deletion packages/image_picker/image_picker_macos/example/macos/Runner.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
archiveVersion = 1;
classes = {
};
objectVersion = 60;
objectVersion = 54;
objects = {

/* Begin PBXAggregateTarget section */
Expand Down
66 changes: 59 additions & 7 deletions packages/image_picker/image_picker_macos/lib/image_picker_macos.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'package:file_selector_macos/file_selector_macos.dart';
import 'package:file_selector_platform_interface/file_selector_platform_interface.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/services.dart';
import 'package:image_picker_platform_interface/image_picker_platform_interface.dart';

import 'src/messages.g.dart';
Expand Down Expand Up @@ -47,8 +48,6 @@ class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform {
/// Supports picking an image, multi-image, video, media, and multiple media.
bool useMacOSPHPicker = false;

// TODO(EchoEllet): avoid using @visibleForTesting per https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md#avoid-using-visiblefortesting

/// Return `true` if the current macOS version supports [useMacOSPHPicker].
///
/// The [PHPicker](https://developer.apple.com/documentation/photokit/phpickerviewcontroller)
Expand Down Expand Up @@ -144,6 +143,8 @@ class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform {
_imageOptionsToImageSelectionOptions(options),
GeneralOptions(limit: 1),
))
.getSuccessOrThrow()
.filePaths
.firstOrNull;
if (imagePath == null) {
return null;
Expand Down Expand Up @@ -183,7 +184,10 @@ class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform {
case ImageSource.gallery:
if (await shouldUsePHPicker()) {
final String? videoPath =
(await _hostApi.pickVideos(GeneralOptions(limit: 1))).firstOrNull;
(await _hostApi.pickVideos(GeneralOptions(limit: 1)))
.getSuccessOrThrow()
.filePaths
.firstOrNull;
if (videoPath == null) {
return null;
}
Expand Down Expand Up @@ -228,12 +232,14 @@ class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform {
MultiImagePickerOptions options = const MultiImagePickerOptions(),
}) async {
if (await shouldUsePHPicker()) {
final List<String> images = await _hostApi.pickImages(
final List<String> images = (await _hostApi.pickImages(
_imageOptionsToImageSelectionOptions(options.imageOptions),
GeneralOptions(
limit: options.limit ?? 0,
),
);
))
.getSuccessOrThrow()
.filePaths;
return images.map((String imagePath) => XFile(imagePath)).toList();
}
const XTypeGroup typeGroup =
Expand Down Expand Up @@ -267,15 +273,17 @@ class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform {
@override
Future<List<XFile>> getMedia({required MediaOptions options}) async {
if (await shouldUsePHPicker()) {
final List<String> images = await _hostApi.pickMedia(
final List<String> images = (await _hostApi.pickMedia(
MediaSelectionOptions(
imageSelectionOptions:
_imageOptionsToImageSelectionOptions(options.imageOptions),
),
GeneralOptions(
limit: options.limit ?? (options.allowMultiple ? 0 : 1),
),
);
))
.getSuccessOrThrow()
.filePaths;
return images.map((String mediaPath) => XFile(mediaPath)).toList();
}
const XTypeGroup typeGroup = XTypeGroup(
Expand All @@ -297,3 +305,47 @@ class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform {
return files;
}
}

extension _ImagePickerResultExt on ImagePickerResult {
/// Returns the result as an [ImagePickerSuccessResult], or throws a [PlatformException]
/// if the result is an [ImagePickerErrorResult].
ImagePickerSuccessResult getSuccessOrThrow() {
final ImagePickerResult result = this;
return switch (result) {
ImagePickerSuccessResult() => result,
ImagePickerErrorResult() => () {
final String errorMessage = switch (result.error) {
ImagePickerError.phpickerUnsupported =>
'PHPicker is only supported on macOS 13.0 or newer.',
ImagePickerError.windowNotFound =>
'No active window to present the picker.',
ImagePickerError.invalidImageSelection =>
'One of the selected items is not an image.',
ImagePickerError.invalidVideoSelection =>
'One of the selected items is not a video.',
ImagePickerError.imageLoadFailed =>
'An error occurred while loading the image.',
ImagePickerError.videoLoadFailed =>
'An error occurred while loading the video.',
ImagePickerError.imageConversionFailed =>
'Failed to convert the NSImage to TIFF data.',
ImagePickerError.imageSaveFailed =>
'Error saving the NSImage data to a file.',
ImagePickerError.imageCompressionFailed =>
'Error while compressing the Data of the NSImage.',
ImagePickerError.multiVideoSelectionUnsupported =>
'The multi-video selection is not supported.',
};
// TODO(EchoEllet): Replace PlatformException with a plugin-specific exception.
// This is currently implemented to maintain compatibility with the existing behavior
// of other implementations of `image_picker`. For more details, refer to:
// https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#platform-exception-handling
throw PlatformException(
code: result.error.name,
message: errorMessage,
details: result.platformErrorMessage,
);
}(),
};
}
}
137 changes: 123 additions & 14 deletions packages/image_picker/image_picker_macos/lib/src/messages.g.dart
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// 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.
// Autogenerated from Pigeon (v22.6.1), do not edit directly.
// Autogenerated from Pigeon (v22.7.2), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import, no_leading_underscores_for_local_identifiers

Expand Down Expand Up @@ -29,6 +29,45 @@ List<Object?> wrapResponse(
return <Object?>[error.code, error.message, error.details];
}

/// Possible error conditions for [ImagePickerApi] calls.
enum ImagePickerError {
/// The current macOS version doesn't support [PHPickerViewController](https://developer.apple.com/documentation/photosui/phpickerviewcontroller)
/// which is supported on macOS 13+.
phpickerUnsupported,

/// Could not show the picker due to the missing window.
windowNotFound,

/// When a `PHPickerResult` can't load `NSImage`. This error should not be reached
/// as the filter in the `PHPickerConfiguration` is set to accept only images.
invalidImageSelection,

/// When a `PHPickerResult` is not a video. This error should not be reached
/// as the filter in the `PHPickerConfiguration` is set to accept only videos.
invalidVideoSelection,

/// Could not load the image object as `NSImage`.
imageLoadFailed,

/// Could not load the video data representation.
videoLoadFailed,

/// The image tiff representation could not be loaded from the `NSImage`.
imageConversionFailed,

/// The loaded `Data` from the `NSImage` could not be written as a file.
imageSaveFailed,

/// The image could not be compressed or the `NSImage` could not be created
/// from the compressed `Data`.
imageCompressionFailed,

/// The multi-video selection is not supported as it's not supported in
/// the app-facing package (`pickVideos` is missing).
/// The multi-video selection is supported when using `pickMedia` instead.
multiVideoSelectionUnsupported,
}

/// The common options between [ImageSelectionOptions], [VideoSelectionOptions]
/// and [MediaSelectionOptions].
class GeneralOptions {
Expand Down Expand Up @@ -132,24 +171,85 @@ class MediaSelectionOptions {
}
}

sealed class ImagePickerResult {}

class ImagePickerSuccessResult extends ImagePickerResult {
ImagePickerSuccessResult({
required this.filePaths,
});

/// The temporary file paths as a result of picking the images and/or videos.
List<String> filePaths;

Object encode() {
return <Object?>[
filePaths,
];
}

static ImagePickerSuccessResult decode(Object result) {
result as List<Object?>;
return ImagePickerSuccessResult(
filePaths: (result[0] as List<Object?>?)!.cast<String>(),
);
}
}

class ImagePickerErrorResult extends ImagePickerResult {
ImagePickerErrorResult({
required this.error,
this.platformErrorMessage,
});

/// Potential error conditions for [ImagePickerApi] calls.
ImagePickerError error;

/// Additional error message from the platform side.
String? platformErrorMessage;

Object encode() {
return <Object?>[
error,
platformErrorMessage,
];
}

static ImagePickerErrorResult decode(Object result) {
result as List<Object?>;
return ImagePickerErrorResult(
error: result[0]! as ImagePickerError,
platformErrorMessage: result[1] as String?,
);
}
}

class _PigeonCodec extends StandardMessageCodec {
const _PigeonCodec();
@override
void writeValue(WriteBuffer buffer, Object? value) {
if (value is int) {
buffer.putUint8(4);
buffer.putInt64(value);
} else if (value is GeneralOptions) {
} else if (value is ImagePickerError) {
buffer.putUint8(129);
writeValue(buffer, value.index);
} else if (value is GeneralOptions) {
buffer.putUint8(130);
writeValue(buffer, value.encode());
} else if (value is MaxSize) {
buffer.putUint8(130);
buffer.putUint8(131);
writeValue(buffer, value.encode());
} else if (value is ImageSelectionOptions) {
buffer.putUint8(131);
buffer.putUint8(132);
writeValue(buffer, value.encode());
} else if (value is MediaSelectionOptions) {
buffer.putUint8(132);
buffer.putUint8(133);
writeValue(buffer, value.encode());
} else if (value is ImagePickerSuccessResult) {
buffer.putUint8(134);
writeValue(buffer, value.encode());
} else if (value is ImagePickerErrorResult) {
buffer.putUint8(135);
writeValue(buffer, value.encode());
} else {
super.writeValue(buffer, value);
Expand All @@ -160,13 +260,20 @@ class _PigeonCodec extends StandardMessageCodec {
Object? readValueOfType(int type, ReadBuffer buffer) {
switch (type) {
case 129:
return GeneralOptions.decode(readValue(buffer)!);
final int? value = readValue(buffer) as int?;
return value == null ? null : ImagePickerError.values[value];
case 130:
return MaxSize.decode(readValue(buffer)!);
return GeneralOptions.decode(readValue(buffer)!);
case 131:
return ImageSelectionOptions.decode(readValue(buffer)!);
return MaxSize.decode(readValue(buffer)!);
case 132:
return ImageSelectionOptions.decode(readValue(buffer)!);
case 133:
return MediaSelectionOptions.decode(readValue(buffer)!);
case 134:
return ImagePickerSuccessResult.decode(readValue(buffer)!);
case 135:
return ImagePickerErrorResult.decode(readValue(buffer)!);
default:
return super.readValueOfType(type, buffer);
}
Expand All @@ -188,6 +295,8 @@ class ImagePickerApi {

final String pigeonVar_messageChannelSuffix;

/// Returns whether [PHPickerViewController](https://developer.apple.com/documentation/photosui/phpickerviewcontroller)
/// is supported on the current macOS version.
Future<bool> supportsPHPicker() async {
final String pigeonVar_channelName =
'dev.flutter.pigeon.image_picker_macos.ImagePickerApi.supportsPHPicker$pigeonVar_messageChannelSuffix';
Expand Down Expand Up @@ -217,7 +326,7 @@ class ImagePickerApi {
}
}

Future<List<String>> pickImages(
Future<ImagePickerResult> pickImages(
ImageSelectionOptions options, GeneralOptions generalOptions) async {
final String pigeonVar_channelName =
'dev.flutter.pigeon.image_picker_macos.ImagePickerApi.pickImages$pigeonVar_messageChannelSuffix';
Expand All @@ -243,12 +352,12 @@ class ImagePickerApi {
message: 'Host platform returned null value for non-null return value.',
);
} else {
return (pigeonVar_replyList[0] as List<Object?>?)!.cast<String>();
return (pigeonVar_replyList[0] as ImagePickerResult?)!;
}
}

/// Currently, multi-video selection is unimplemented.
Future<List<String>> pickVideos(GeneralOptions generalOptions) async {
Future<ImagePickerResult> pickVideos(GeneralOptions generalOptions) async {
final String pigeonVar_channelName =
'dev.flutter.pigeon.image_picker_macos.ImagePickerApi.pickVideos$pigeonVar_messageChannelSuffix';
final BasicMessageChannel<Object?> pigeonVar_channel =
Expand All @@ -273,11 +382,11 @@ class ImagePickerApi {
message: 'Host platform returned null value for non-null return value.',
);
} else {
return (pigeonVar_replyList[0] as List<Object?>?)!.cast<String>();
return (pigeonVar_replyList[0] as ImagePickerResult?)!;
}
}

Future<List<String>> pickMedia(
Future<ImagePickerResult> pickMedia(
MediaSelectionOptions options, GeneralOptions generalOptions) async {
final String pigeonVar_channelName =
'dev.flutter.pigeon.image_picker_macos.ImagePickerApi.pickMedia$pigeonVar_messageChannelSuffix';
Expand All @@ -303,7 +412,7 @@ class ImagePickerApi {
message: 'Host platform returned null value for non-null return value.',
);
} else {
return (pigeonVar_replyList[0] as List<Object?>?)!.cast<String>();
return (pigeonVar_replyList[0] as ImagePickerResult?)!;
}
}
}
Loading

0 comments on commit 2a101a9

Please sign in to comment.