Browse Source

fix(mobile): validate response code from download file (#4543)

Co-authored-by: shalong-tanwen <139912620+shalong-tanwen@users.noreply.github.com>
shenlong 1 year ago
parent
commit
22172a680b

+ 1 - 0
mobile/assets/i18n/en-US.json

@@ -164,6 +164,7 @@
   "home_page_favorite_err_local": "Can not favorite local assets yet, skipping",
   "home_page_favorite_err_local": "Can not favorite local assets yet, skipping",
   "home_page_first_time_notice": "If this is your first time using the app, please make sure to choose a backup album(s) so that the timeline can populate photos and videos in the album(s).",
   "home_page_first_time_notice": "If this is your first time using the app, please make sure to choose a backup album(s) so that the timeline can populate photos and videos in the album(s).",
   "image_viewer_page_state_provider_download_error": "Download Error",
   "image_viewer_page_state_provider_download_error": "Download Error",
+  "image_viewer_page_state_provider_share_error": "Share Error",
   "image_viewer_page_state_provider_download_success": "Download Success",
   "image_viewer_page_state_provider_download_success": "Download Success",
   "library_page_albums": "Albums",
   "library_page_albums": "Albums",
   "library_page_archive": "Archive",
   "library_page_archive": "Archive",

+ 13 - 3
mobile/lib/modules/asset_viewer/providers/image_viewer_page_state.provider.dart

@@ -57,9 +57,19 @@ class ImageViewerStateNotifier extends StateNotifier<ImageViewerPageState> {
     showDialog(
     showDialog(
       context: context,
       context: context,
       builder: (BuildContext buildContext) {
       builder: (BuildContext buildContext) {
-        _shareService
-            .shareAsset(asset)
-            .then((_) => Navigator.of(buildContext).pop());
+        _shareService.shareAsset(asset).then(
+          (bool status) {
+            if (!status) {
+              ImmichToast.show(
+                context: context,
+                msg: 'image_viewer_page_state_provider_share_error'.tr(),
+                toastType: ToastType.error,
+                gravity: ToastGravity.BOTTOM,
+              );
+            }
+            Navigator.of(buildContext).pop();
+          },
+        );
         return const ShareDialog();
         return const ShareDialog();
       },
       },
       barrierDismissible: false,
       barrierDismissible: false,

+ 19 - 0
mobile/lib/modules/asset_viewer/services/image_viewer.service.dart

@@ -5,6 +5,7 @@ import 'package:hooks_riverpod/hooks_riverpod.dart';
 import 'package:immich_mobile/shared/models/asset.dart';
 import 'package:immich_mobile/shared/models/asset.dart';
 import 'package:immich_mobile/shared/providers/api.provider.dart';
 import 'package:immich_mobile/shared/providers/api.provider.dart';
 import 'package:immich_mobile/shared/services/api.service.dart';
 import 'package:immich_mobile/shared/services/api.service.dart';
+import 'package:logging/logging.dart';
 
 
 import 'package:photo_manager/photo_manager.dart';
 import 'package:photo_manager/photo_manager.dart';
 import 'package:path_provider/path_provider.dart';
 import 'package:path_provider/path_provider.dart';
@@ -14,6 +15,7 @@ final imageViewerServiceProvider =
 
 
 class ImageViewerService {
 class ImageViewerService {
   final ApiService _apiService;
   final ApiService _apiService;
+  final Logger _log = Logger("ImageViewerService");
 
 
   ImageViewerService(this._apiService);
   ImageViewerService(this._apiService);
 
 
@@ -29,6 +31,16 @@ class ImageViewerService {
           asset.livePhotoVideoId!,
           asset.livePhotoVideoId!,
         );
         );
 
 
+        if (imageResponse.statusCode != 200 ||
+            motionReponse.statusCode != 200) {
+          final failedResponse =
+              imageResponse.statusCode != 200 ? imageResponse : motionReponse;
+          _log.severe(
+            "Motion asset download failed with status - ${failedResponse.statusCode} and response - ${failedResponse.body}",
+          );
+          return false;
+        }
+
         final AssetEntity? entity;
         final AssetEntity? entity;
 
 
         final tempDir = await getTemporaryDirectory();
         final tempDir = await getTemporaryDirectory();
@@ -48,6 +60,13 @@ class ImageViewerService {
         var res = await _apiService.assetApi
         var res = await _apiService.assetApi
             .downloadFileWithHttpInfo(asset.remoteId!);
             .downloadFileWithHttpInfo(asset.remoteId!);
 
 
+        if (res.statusCode != 200) {
+          _log.severe(
+            "Asset download failed with status - ${res.statusCode} and response - ${res.body}",
+          );
+          return false;
+        }
+
         final AssetEntity? entity;
         final AssetEntity? entity;
 
 
         if (asset.isImage) {
         if (asset.isImage) {

+ 49 - 20
mobile/lib/shared/services/share.service.dart

@@ -4,6 +4,7 @@ import 'package:flutter/material.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
 import 'package:immich_mobile/shared/models/asset.dart';
 import 'package:immich_mobile/shared/models/asset.dart';
 import 'package:immich_mobile/shared/providers/api.provider.dart';
 import 'package:immich_mobile/shared/providers/api.provider.dart';
+import 'package:logging/logging.dart';
 import 'package:path_provider/path_provider.dart';
 import 'package:path_provider/path_provider.dart';
 import 'package:share_plus/share_plus.dart';
 import 'package:share_plus/share_plus.dart';
 import 'api.service.dart';
 import 'api.service.dart';
@@ -13,32 +14,60 @@ final shareServiceProvider =
 
 
 class ShareService {
 class ShareService {
   final ApiService _apiService;
   final ApiService _apiService;
+  final Logger _log = Logger("ShareService");
 
 
   ShareService(this._apiService);
   ShareService(this._apiService);
 
 
-  Future<void> shareAsset(Asset asset) async {
-    await shareAssets([asset]);
+  Future<bool> shareAsset(Asset asset) async {
+    return await shareAssets([asset]);
   }
   }
 
 
-  Future<void> shareAssets(List<Asset> assets) async {
-    final downloadedXFiles = assets.map<Future<XFile>>((asset) async {
-      if (asset.isRemote) {
-        final tempDir = await getTemporaryDirectory();
-        final fileName = asset.fileName;
-        final tempFile = await File('${tempDir.path}/$fileName').create();
-        final res = await _apiService.assetApi
-            .downloadFileWithHttpInfo(asset.remoteId!);
-        tempFile.writeAsBytesSync(res.bodyBytes);
-        return XFile(tempFile.path);
-      } else {
-        File? f = await asset.local!.file;
-        return XFile(f!.path);
+  Future<bool> shareAssets(List<Asset> assets) async {
+    try {
+      final downloadedXFiles = <XFile>[];
+
+      for (var asset in assets) {
+        if (asset.isRemote) {
+          final tempDir = await getTemporaryDirectory();
+          final fileName = asset.fileName;
+          final tempFile = await File('${tempDir.path}/$fileName').create();
+          final res = await _apiService.assetApi
+              .downloadFileWithHttpInfo(asset.remoteId!);
+
+          if (res.statusCode != 200) {
+            _log.severe(
+              "Asset download failed with status - ${res.statusCode} and response - ${res.body}",
+            );
+            continue;
+          }
+
+          tempFile.writeAsBytesSync(res.bodyBytes);
+          downloadedXFiles.add(XFile(tempFile.path));
+        } else {
+          File? f = await asset.local!.file;
+          downloadedXFiles.add(XFile(f!.path));
+        }
+      }
+
+      if (downloadedXFiles.isEmpty) {
+        _log.warning("No asset can be retrieved for share");
+        return false;
+      }
+
+      if (downloadedXFiles.length != assets.length) {
+        _log.warning(
+          "Partial share - Requested: ${assets.length}, Sharing: ${downloadedXFiles.length}",
+        );
       }
       }
-    });
 
 
-    Share.shareXFiles(
-      await Future.wait(downloadedXFiles),
-      sharePositionOrigin: Rect.zero,
-    );
+      Share.shareXFiles(
+        downloadedXFiles,
+        sharePositionOrigin: Rect.zero,
+      );
+      return true;
+    } catch (error) {
+      _log.severe("Share failed with error $error");
+    }
+    return false;
   }
   }
 }
 }

+ 14 - 4
mobile/lib/utils/selection_handlers.dart

@@ -1,3 +1,4 @@
+import 'package:easy_localization/easy_localization.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/material.dart';
 import 'package:fluttertoast/fluttertoast.dart';
 import 'package:fluttertoast/fluttertoast.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
@@ -15,10 +16,19 @@ void handleShareAssets(
   showDialog(
   showDialog(
     context: context,
     context: context,
     builder: (BuildContext buildContext) {
     builder: (BuildContext buildContext) {
-      ref
-          .watch(shareServiceProvider)
-          .shareAssets(selection.toList())
-          .then((_) => Navigator.of(buildContext).pop());
+      ref.watch(shareServiceProvider).shareAssets(selection.toList()).then(
+        (bool status) {
+          if (!status) {
+            ImmichToast.show(
+              context: context,
+              msg: 'image_viewer_page_state_provider_share_error'.tr(),
+              toastType: ToastType.error,
+              gravity: ToastGravity.BOTTOM,
+            );
+          }
+          Navigator.of(buildContext).pop();
+        },
+      );
       return const ShareDialog();
       return const ShareDialog();
     },
     },
     barrierDismissible: false,
     barrierDismissible: false,