fix: 3595 - transient file refactoring and fixes (#3939)

New file: * `background_task_upload.dart`: Background task about generic file upload. Impacted files: * `abstract_background_task.dart`: added a `recover` abstract method * `background_task_crop.dart`: now extends new class `BackgroundTaskUpload` * `background_task_image.dart`: now extends new class `BackgroundTaskUpload` * `background_task_manager.dart`: now calls `recover`for each task at run time; minor fix about "internet error" * `background_task_unselect.dart`: minor refactoring; minor fix removing the transient file * `crop_page.dart`: minor refactoring * `edit_ingredients_page.dart`: minor refactoring * `image_upload_card.dart`: minor refactoring * `product_cards_helper.dart`: minor refactoring * `product_image_viewer.dart`: minor refactoring * `smooth_product_image.dart`: minor refactoring * `transient_file.dart`: refactored as a "real" class, not a static one
openfoodfacts · May 13, 2023 · 6e72ff0 · 6e72ff0
1 parent 72f3396
commit 6e72ff0
Show file tree

Hide file tree

Showing 13 changed files with 196 additions and 188 deletions.
diff --git a/packages/smooth_app/lib/background/abstract_background_task.dart b/packages/smooth_app/lib/background/abstract_background_task.dart
@@ -133,4 +133,12 @@ abstract class AbstractBackgroundTask {
         barcode: barcode,
         localDatabase: localDatabase,
       );
+
+  /// Checks that everything is fine and fix things if needed + if possible.
+  ///
+  /// To be run systematically for each task.
+  /// Especially useful for transient files: if a user closed the app before
+  /// successfully completing the upload task, the transient file - that is just
+  /// a static variable - won't be there at app restart. Unless you recover.
+  Future<void> recover(final LocalDatabase localDatabase) async {}
 }
diff --git a/packages/smooth_app/lib/background/background_task_crop.dart b/packages/smooth_app/lib/background/background_task_crop.dart
@@ -6,15 +6,14 @@ import 'package:flutter_gen/gen_l10n/app_localizations.dart';
 import 'package:openfoodfacts/openfoodfacts.dart';
 import 'package:provider/provider.dart';
 import 'package:smooth_app/background/abstract_background_task.dart';
-import 'package:smooth_app/background/background_task_image.dart';
 import 'package:smooth_app/background/background_task_refresh_later.dart';
+import 'package:smooth_app/background/background_task_upload.dart';
 import 'package:smooth_app/data_models/operation_type.dart';
 import 'package:smooth_app/database/local_database.dart';
-import 'package:smooth_app/database/transient_file.dart';
 import 'package:smooth_app/query/product_query.dart';
 
 /// Background task about product image crop from existing file.
-class BackgroundTaskCrop extends AbstractBackgroundTask {
+class BackgroundTaskCrop extends BackgroundTaskUpload {
   const BackgroundTaskCrop._({
     required super.processName,
     required super.uniqueId,
@@ -23,14 +22,14 @@ class BackgroundTaskCrop extends AbstractBackgroundTask {
     required super.user,
     required super.country,
     required super.stamp,
+    required super.imageField,
+    required super.croppedPath,
+    required super.rotationDegrees,
+    required super.cropX1,
+    required super.cropY1,
+    required super.cropX2,
+    required super.cropY2,
     required this.imageId,
-    required this.imageField,
-    required this.croppedPath,
-    required this.rotationDegrees,
-    required this.cropX1,
-    required this.cropY1,
-    required this.cropX2,
-    required this.cropY2,
   });
 
   BackgroundTaskCrop._fromJson(Map<String, dynamic> json)
@@ -58,13 +57,6 @@ class BackgroundTaskCrop extends AbstractBackgroundTask {
   static const OperationType _operationType = OperationType.crop;
 
   final int imageId;
-  final String imageField;
-  final String croppedPath;
-  final int rotationDegrees;
-  final int cropX1;
-  final int cropY1;
-  final int cropX2;
-  final int cropY2;
 
   @override
   Map<String, dynamic> toJson() => <String, dynamic>{
@@ -166,7 +158,7 @@ class BackgroundTaskCrop extends AbstractBackgroundTask {
         languageCode: language.code,
         user: jsonEncode(ProductQuery.getUser().toJson()),
         country: ProductQuery.getCountry()!.offTag,
-        stamp: BackgroundTaskImage.getStamp(
+        stamp: BackgroundTaskUpload.getStamp(
           barcode,
           imageField.offTag,
           language.code,
@@ -182,13 +174,7 @@ class BackgroundTaskCrop extends AbstractBackgroundTask {
         images: <ProductImage>[_getProductImage()],
       ),
     );
-    TransientFile.putImage(
-      ImageField.fromOffTag(imageField)!,
-      barcode,
-      getLanguage(),
-      localDatabase,
-      File(croppedPath),
-    );
+    putTransientImage(localDatabase);
   }
 
   /// Returns the actual crop parameters.
@@ -218,13 +204,7 @@ class BackgroundTaskCrop extends AbstractBackgroundTask {
     } catch (e) {
       // not likely, but let's not spoil the task for that either.
     }
-    TransientFile.removeImage(
-      ImageField.fromOffTag(imageField)!,
-      barcode,
-      getLanguage(),
-      localDatabase,
-    );
-    localDatabase.notifyListeners();
+    removeTransientImage(localDatabase);
     if (success) {
       await BackgroundTaskRefreshLater.addTask(
         barcode,

diff --git a/packages/smooth_app/lib/background/background_task_image.dart b/packages/smooth_app/lib/background/background_task_image.dart
@@ -11,15 +11,15 @@ import 'package:openfoodfacts/openfoodfacts.dart';
 import 'package:provider/provider.dart';
 import 'package:smooth_app/background/abstract_background_task.dart';
 import 'package:smooth_app/background/background_task_refresh_later.dart';
+import 'package:smooth_app/background/background_task_upload.dart';
 import 'package:smooth_app/data_models/operation_type.dart';
 import 'package:smooth_app/data_models/up_to_date_changes.dart';
 import 'package:smooth_app/database/local_database.dart';
-import 'package:smooth_app/database/transient_file.dart';
 import 'package:smooth_app/helpers/image_compute_container.dart';
 import 'package:smooth_app/query/product_query.dart';
 
 /// Background task about product image upload.
-class BackgroundTaskImage extends AbstractBackgroundTask {
+class BackgroundTaskImage extends BackgroundTaskUpload {
   const BackgroundTaskImage._({
     required super.processName,
     required super.uniqueId,
@@ -28,14 +28,14 @@ class BackgroundTaskImage extends AbstractBackgroundTask {
     required super.user,
     required super.country,
     required super.stamp,
-    required this.imageField,
+    required super.imageField,
+    required super.croppedPath,
+    required super.rotationDegrees,
+    required super.cropX1,
+    required super.cropY1,
+    required super.cropX2,
+    required super.cropY2,
     required this.fullPath,
-    required this.croppedPath,
-    required this.rotationDegrees,
-    required this.cropX1,
-    required this.cropY1,
-    required this.cropX2,
-    required this.cropY2,
   });
 
   BackgroundTaskImage._fromJson(Map<String, dynamic> json)
@@ -59,7 +59,7 @@ class BackgroundTaskImage extends AbstractBackgroundTask {
           // dealing with when 'stamp' did not exist
           stamp: json.containsKey('stamp')
               ? json['stamp'] as String
-              : getStamp(
+              : BackgroundTaskUpload.getStamp(
                   json['barcode'] as String,
                   json['imageField'] as String,
                   json['languageCode'] as String,
@@ -71,14 +71,7 @@ class BackgroundTaskImage extends AbstractBackgroundTask {
 
   static const OperationType _operationType = OperationType.image;
 
-  final String imageField;
   final String fullPath;
-  final String croppedPath;
-  final int rotationDegrees;
-  final int cropX1;
-  final int cropY1;
-  final int cropX2;
-  final int cropY2;
 
   @override
   Map<String, dynamic> toJson() => <String, dynamic>{
@@ -180,20 +173,13 @@ class BackgroundTaskImage extends AbstractBackgroundTask {
         languageCode: language.code,
         user: jsonEncode(ProductQuery.getUser().toJson()),
         country: ProductQuery.getCountry()!.offTag,
-        stamp: getStamp(
+        stamp: BackgroundTaskUpload.getStamp(
           barcode,
           imageField.offTag,
           language.code,
         ),
       );
 
-  static String getStamp(
-    final String barcode,
-    final String imageField,
-    final String language,
-  ) =>
-      '$barcode;image;$imageField;$language';
-
   /// Returns true if the stamp is an "image/OTHER" stamp.
   ///
   /// That's important because "image/OTHER" task are never duplicates.
@@ -209,13 +195,7 @@ class BackgroundTaskImage extends AbstractBackgroundTask {
         images: <ProductImage>[_getProductImage()],
       ),
     );
-    TransientFile.putImage(
-      ImageField.fromOffTag(imageField)!,
-      barcode,
-      getLanguage(),
-      localDatabase,
-      File(croppedPath),
-    );
+    putTransientImage(localDatabase);
   }
 
   /// Returns a fake value that means: "remove the previous value when merging".
@@ -252,13 +232,7 @@ class BackgroundTaskImage extends AbstractBackgroundTask {
     } catch (e) {
       // possible, but let's not spoil the task for that either.
     }
-    TransientFile.removeImage(
-      ImageField.fromOffTag(imageField)!,
-      barcode,
-      getLanguage(),
-      localDatabase,
-    );
-    localDatabase.notifyListeners();
+    removeTransientImage(localDatabase);
     if (success) {
       await BackgroundTaskRefreshLater.addTask(
         barcode,

diff --git a/packages/smooth_app/lib/background/background_task_manager.dart b/packages/smooth_app/lib/background/background_task_manager.dart
@@ -134,6 +134,9 @@ class BackgroundTaskManager {
       return;
     }
     final List<AbstractBackgroundTask> tasks = await _getAllTasks();
+    for (final AbstractBackgroundTask task in tasks) {
+      await task.recover(localDatabase);
+    }
     final Map<String, String> failedTaskFromStamps = <String, String>{};
     for (final AbstractBackgroundTask task in tasks) {
       final String stamp = task.stamp;
@@ -156,6 +159,7 @@ class BackgroundTaskManager {
         // Most likely, no internet, no reason to go on.
         if (e.toString().startsWith('Failed host lookup: ')) {
           await _setTaskErrorStatus(taskId, taskStatusNoInternet);
+          await _justFinished();
           return;
         }
         debugPrint('Background task error ($e)');

diff --git a/packages/smooth_app/lib/background/background_task_unselect.dart b/packages/smooth_app/lib/background/background_task_unselect.dart
@@ -5,10 +5,11 @@ import 'package:flutter_gen/gen_l10n/app_localizations.dart';
 import 'package:openfoodfacts/openfoodfacts.dart';
 import 'package:provider/provider.dart';
 import 'package:smooth_app/background/abstract_background_task.dart';
-import 'package:smooth_app/background/background_task_image.dart';
 import 'package:smooth_app/background/background_task_refresh_later.dart';
+import 'package:smooth_app/background/background_task_upload.dart';
 import 'package:smooth_app/data_models/operation_type.dart';
 import 'package:smooth_app/database/local_database.dart';
+import 'package:smooth_app/database/transient_file.dart';
 import 'package:smooth_app/helpers/image_field_extension.dart';
 import 'package:smooth_app/query/product_query.dart';
 
@@ -111,16 +112,24 @@ class BackgroundTaskUnselect extends AbstractBackgroundTask {
         user: jsonEncode(ProductQuery.getUser().toJson()),
         country: ProductQuery.getCountry()!.offTag,
         // same stamp as image upload
-        stamp: BackgroundTaskImage.getStamp(
+        stamp: BackgroundTaskUpload.getStamp(
           barcode,
           imageField.offTag,
           language.code,
         ),
       );
 
   @override
-  Future<void> preExecute(final LocalDatabase localDatabase) async =>
-      localDatabase.upToDate.addChange(uniqueId, _getUnselectedProduct());
+  Future<void> preExecute(final LocalDatabase localDatabase) async {
+    localDatabase.upToDate.addChange(uniqueId, _getUnselectedProduct());
+    _getTransientFile().removeImage(localDatabase);
+  }
+
+  TransientFile _getTransientFile() => TransientFile(
+        ImageField.fromOffTag(imageField)!,
+        barcode,
+        getLanguage(),
+      );
 
   @override
   Future<void> postExecute(

diff --git a/packages/smooth_app/lib/background/background_task_upload.dart b/packages/smooth_app/lib/background/background_task_upload.dart
@@ -0,0 +1,69 @@
+import 'dart:async';
+import 'dart:io';
+import 'package:flutter/material.dart';
+import 'package:openfoodfacts/openfoodfacts.dart';
+import 'package:smooth_app/background/abstract_background_task.dart';
+import 'package:smooth_app/database/local_database.dart';
+import 'package:smooth_app/database/transient_file.dart';
+
+/// Background task about generic file upload.
+abstract class BackgroundTaskUpload extends AbstractBackgroundTask {
+  const BackgroundTaskUpload({
+    required super.processName,
+    required super.uniqueId,
+    required super.barcode,
+    required super.languageCode,
+    required super.user,
+    required super.country,
+    required super.stamp,
+    required this.imageField,
+    required this.croppedPath,
+    required this.rotationDegrees,
+    required this.cropX1,
+    required this.cropY1,
+    required this.cropX2,
+    required this.cropY2,
+  });
+
+  final String imageField;
+  final String croppedPath;
+  final int rotationDegrees;
+  final int cropX1;
+  final int cropY1;
+  final int cropX2;
+  final int cropY2;
+
+  TransientFile _getTransientFile() => TransientFile(
+        ImageField.fromOffTag(imageField)!,
+        barcode,
+        getLanguage(),
+      );
+
+  @protected
+  void putTransientImage(final LocalDatabase localDatabase) =>
+      _getTransientFile().putImage(
+        localDatabase,
+        File(croppedPath),
+      );
+
+  @protected
+  void removeTransientImage(final LocalDatabase localDatabase) =>
+      _getTransientFile().removeImage(localDatabase);
+
+  File? _getTransientImage() => _getTransientFile().getImage();
+
+  @override
+  Future<void> recover(final LocalDatabase localDatabase) async {
+    final File? transientFile = _getTransientImage();
+    if (transientFile == null) {
+      putTransientImage(localDatabase);
+    }
+  }
+
+  static String getStamp(
+    final String barcode,
+    final String imageField,
+    final String language,
+  ) =>
+      '$barcode;image;$imageField;$language';
+}
diff --git a/packages/smooth_app/lib/cards/data_cards/image_upload_card.dart b/packages/smooth_app/lib/cards/data_cards/image_upload_card.dart
@@ -34,11 +34,11 @@ class _ImageUploadCardState extends State<ImageUploadCard> {
     final Size screenSize = MediaQuery.of(context).size;
     final AppLocalizations appLocalizations = AppLocalizations.of(context);
     context.watch<LocalDatabase>();
-    final ImageProvider? imageProvider = TransientFile.getImageProvider(
+    final ImageProvider? imageProvider = TransientFile.fromProductImageData(
       widget.productImageData,
       widget.product.barcode!,
       ProductQuery.getLanguage(),
-    );
+    ).getImageProvider();
 
     if (imageProvider == null) {
       return ElevatedButton.icon(