#94 Downloads failing immediately correctly update UI

Add a Lock to DownloadService. This ensures that download status updates are not processed while a download start/stop is requested (they are processed right after).

Author: Chralu

lib/bloc/podcast/episode_bloc.dart

@@ -7,6 +7,7 @@ import 'dart:async';
 import 'package:anytime/bloc/bloc.dart';
 import 'package:anytime/entities/episode.dart';
 import 'package:anytime/services/audio/audio_player_service.dart';
+import 'package:anytime/services/download/download_service.dart';
 import 'package:anytime/services/podcast/podcast_service.dart';
 import 'package:anytime/state/bloc_state.dart';
 import 'package:logging/logging.dart';
@@ -17,6 +18,7 @@ import 'package:rxdart/rxdart.dart';
 class EpisodeBloc extends Bloc {
   final log = Logger('EpisodeBloc');
   final PodcastService podcastService;
+  final DownloadService downloadService;
   final AudioPlayerService audioPlayerService;
 
   /// Add to sink to fetch list of current downloaded episodes.
@@ -43,6 +45,7 @@ class EpisodeBloc extends Bloc {
   EpisodeBloc({
     required this.podcastService,
     required this.audioPlayerService,
+    required this.downloadService,
   }) {
     _init();
   }
@@ -65,7 +68,7 @@ class EpisodeBloc extends Bloc {
         await audioPlayerService.stop();
       }
 
-      await podcastService.deleteDownload(episode!);
+      await downloadService.deleteDownload(episode!);
 
       fetchDownloads(true);
     });

lib/services/download/download_service.dart

@@ -6,6 +6,7 @@ import 'package:anytime/entities/episode.dart';
 
 abstract class DownloadService {
   Future<bool> downloadEpisode(Episode episode);
+  Future<void> deleteDownload(Episode episode);
 
   Future<Episode?> findEpisodeByTaskId(String taskId);
 

lib/services/download/mobile_download_service.dart

@@ -11,10 +11,13 @@ import 'package:anytime/entities/episode.dart';
 import 'package:anytime/repository/repository.dart';
 import 'package:anytime/services/download/download_manager.dart';
 import 'package:anytime/services/download/download_service.dart';
+import 'package:anytime/services/settings/settings_service.dart';
 import 'package:collection/collection.dart' show IterableExtension;
+import 'package:flutter_downloader/flutter_downloader.dart';
 import 'package:logging/logging.dart';
 import 'package:mp3_info/mp3_info.dart';
 import 'package:rxdart/rxdart.dart';
+import 'package:synchronized/synchronized.dart';
 
 /// An implementation of a [DownloadService] that handles downloading
 /// of episodes on mobile.
@@ -23,18 +26,33 @@ class MobileDownloadService extends DownloadService {
 
   final log = Logger('MobileDownloadService');
   final Repository repository;
+  final SettingsService settingsService;
   final DownloadManager downloadManager;
-
-  MobileDownloadService({required this.repository, required this.downloadManager}) {
-    downloadManager.downloadProgress.pipe(downloadProgress);
-    downloadProgress.listen((progress) {
-      _updateDownloadProgress(progress);
-    });
+  late final StreamSubscription _downloadProgressSubscription;
+
+  /// Lock ensures we wait for task creation and local save
+  /// before handling subsequent [Download update events].
+  final _downloadProgressLock = Lock();
+
+  MobileDownloadService({
+    required this.repository,
+    required this.downloadManager,
+    required this.settingsService,
+  }) {
+    _downloadProgressSubscription = downloadManager.downloadProgress.listen(
+      (event) async => await _downloadProgressLock.synchronized(
+        () {
+          downloadProgress.add(event);
+          _updateDownloadProgress(event);
+        },
+      ),
+    );
   }
 
   @override
   void dispose() {
     downloadManager.dispose();
+    _downloadProgressSubscription.cancel();
   }
 
   @override
@@ -91,16 +109,22 @@ class MobileDownloadService extends DownloadService {
         /// the URL before calling download and ensure it is https.
         var url = await resolveUrl(episode.contentUrl!, forceHttps: true);
 
-        final taskId = await downloadManager.enqueueTask(url, downloadPath, filename);
+        await _downloadProgressLock.synchronized(() async {
+          final taskId = await downloadManager.enqueueTask(
+            url,
+            downloadPath,
+            filename!,
+          );
 
-        // Update the episode with download data
-        episode.filepath = episodePath;
-        episode.filename = filename;
-        episode.downloadTaskId = taskId;
-        episode.downloadState = DownloadState.downloading;
-        episode.downloadPercentage = 0;
+          // Update the episode with download data
+          episode.filepath = episodePath;
+          episode.filename = filename;
+          episode.downloadTaskId = taskId;
+          episode.downloadState = DownloadState.downloading;
+          episode.downloadPercentage = 0;
 
-        await repository.saveEpisode(episode);
+          await repository.saveEpisode(episode);
+        });
 
         return Future.value(true);
       }
@@ -109,6 +133,41 @@ class MobileDownloadService extends DownloadService {
     return Future.value(false);
   }
 
+  @override
+  Future<void> deleteDownload(Episode episode) async => _downloadProgressLock.synchronized(() async {
+        // If this episode is currently downloading, cancel the download first.
+        if (episode.downloadState == DownloadState.downloaded) {
+          if (settingsService.markDeletedEpisodesAsPlayed) {
+            episode.played = true;
+          }
+        } else if (episode.downloadState == DownloadState.downloading && episode.downloadPercentage! < 100) {
+          await FlutterDownloader.cancel(taskId: episode.downloadTaskId!);
+        }
+
+        episode.downloadTaskId = null;
+        episode.downloadPercentage = 0;
+        episode.position = 0;
+        episode.downloadState = DownloadState.none;
+
+        if (episode.transcriptId != null && episode.transcriptId! > 0) {
+          await repository.deleteTranscriptById(episode.transcriptId!);
+        }
+
+        await repository.saveEpisode(episode);
+
+        if (await hasStoragePermission()) {
+          final f = File.fromUri(Uri.file(await resolvePath(episode)));
+
+          log.fine('Deleting file ${f.path}');
+
+          if (await f.exists()) {
+            f.delete();
+          }
+        }
+
+        return;
+      });
+
   @override
   Future<Episode?> findEpisodeByTaskId(String taskId) {
     return repository.findEpisodeByTaskId(taskId);

lib/services/podcast/mobile_podcast_service.dart

@@ -8,7 +8,6 @@ import 'dart:io';
 import 'package:anytime/api/podcast/podcast_api.dart';
 import 'package:anytime/core/utils.dart';
 import 'package:anytime/entities/chapter.dart';
-import 'package:anytime/entities/downloadable.dart';
 import 'package:anytime/entities/episode.dart';
 import 'package:anytime/entities/funding.dart';
 import 'package:anytime/entities/person.dart';
@@ -22,7 +21,6 @@ import 'package:anytime/state/episode_state.dart';
 import 'package:collection/collection.dart' show IterableExtension;
 import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
-import 'package:flutter_downloader/flutter_downloader.dart';
 import 'package:intl/intl.dart';
 import 'package:logging/logging.dart';
 import 'package:path/path.dart';
@@ -478,41 +476,6 @@ class MobilePodcastService extends PodcastService {
     return repository.findAllEpisodes();
   }
 
-  @override
-  Future<void> deleteDownload(Episode episode) async {
-    // If this episode is currently downloading, cancel the download first.
-    if (episode.downloadState == DownloadState.downloaded) {
-      if (settingsService.markDeletedEpisodesAsPlayed) {
-        episode.played = true;
-      }
-    } else if (episode.downloadState == DownloadState.downloading && episode.downloadPercentage! < 100) {
-      await FlutterDownloader.cancel(taskId: episode.downloadTaskId!);
-    }
-
-    episode.downloadTaskId = null;
-    episode.downloadPercentage = 0;
-    episode.position = 0;
-    episode.downloadState = DownloadState.none;
-
-    if (episode.transcriptId != null && episode.transcriptId! > 0) {
-      await repository.deleteTranscriptById(episode.transcriptId!);
-    }
-
-    await repository.saveEpisode(episode);
-
-    if (await hasStoragePermission()) {
-      final f = File.fromUri(Uri.file(await resolvePath(episode)));
-
-      log.fine('Deleting file ${f.path}');
-
-      if (await f.exists()) {
-        f.delete();
-      }
-    }
-
-    return;
-  }
-
   @override
   Future<void> toggleEpisodePlayed(Episode episode) async {
     episode.played = !episode.played;

lib/services/podcast/podcast_service.dart

@@ -198,8 +198,6 @@ abstract class PodcastService {
 
   Future<Transcript> loadTranscriptByUrl({required TranscriptUrl transcriptUrl});
 
-  Future<void> deleteDownload(Episode episode);
-
   Future<void> toggleEpisodePlayed(Episode episode);
 
   Future<List<Podcast>> subscriptions();

lib/ui/anytime_podcast_app.dart

@@ -78,6 +78,7 @@ class AnytimePodcastApp extends StatefulWidget {
     downloadService = MobileDownloadService(
       repository: repository,
       downloadManager: MobileDownloaderManager(),
+      settingsService: mobileSettingsService,
     );
 
     podcastService = MobilePodcastService(
@@ -149,8 +150,11 @@ class AnytimePodcastAppState extends State<AnytimePodcastApp> {
           dispose: (_, value) => value.dispose(),
         ),
         Provider<EpisodeBloc>(
-          create: (_) =>
-              EpisodeBloc(podcastService: widget.podcastService!, audioPlayerService: widget.audioPlayerService),
+          create: (_) => EpisodeBloc(
+            podcastService: widget.podcastService!,
+            audioPlayerService: widget.audioPlayerService,
+            downloadService: widget.downloadService,
+          ),
           dispose: (_, value) => value.dispose(),
         ),
         Provider<PodcastBloc>(

pubspec.yaml

@@ -53,6 +53,7 @@ dependencies:
   flutter_localizations:
     sdk: flutter
   collection: ^1.17.1
+  synchronized: ^3.1.0
 
 dev_dependencies:
   intl_generator: ^0.4.1