diff --git a/lib/data/bulk_language_download.dart b/lib/data/bulk_language_download.dart new file mode 100644 index 0000000..c168e4a --- /dev/null +++ b/lib/data/bulk_language_download.dart @@ -0,0 +1,63 @@ +import 'dart:collection'; + +const kMaxParallelLanguageDownloads = 4; + +typedef LanguageDownloadFn = Future Function(String languageCode); + +class BulkDownloadResult { + final int successCount; + final int errorCount; + final String lastSuccessCode; + + const BulkDownloadResult({ + required this.successCount, + required this.errorCount, + required this.lastSuccessCode, + }); +} + +/// Download or update [languageCodes] keeping at most [maxConcurrent] in +/// flight at any time. Uses a worker pool: as soon as one download finishes a +/// new one is started, so a slow language never blocks idle slots. +Future downloadLanguagesInParallel( + Iterable languageCodes, { + required LanguageDownloadFn download, + int maxConcurrent = kMaxParallelLanguageDownloads, +}) async { + assert(maxConcurrent > 0); + final queue = Queue.of(languageCodes); + var successCount = 0; + var errorCount = 0; + var lastSuccessCode = ''; + + // Each worker pulls the next code until the queue is empty. Safe without + // locking: there's no await between isNotEmpty and removeFirst, and Dart is + // single-threaded. + Future worker() async { + while (queue.isNotEmpty) { + final code = queue.removeFirst(); + bool success; + try { + success = await download(code); + } catch (_) { + success = false; + } + if (success) { + successCount++; + lastSuccessCode = code; + } else { + errorCount++; + } + } + } + + await Future.wait([ + for (var i = 0; i < maxConcurrent; i++) worker(), + ]); + + return BulkDownloadResult( + successCount: successCount, + errorCount: errorCount, + lastSuccessCode: lastSuccessCode, + ); +} diff --git a/lib/data/language_downloader.dart b/lib/data/language_downloader.dart index fd66ec6..9861eb6 100644 --- a/lib/data/language_downloader.dart +++ b/lib/data/language_downloader.dart @@ -18,7 +18,7 @@ class LanguageDownloaderImpl implements LanguageDownloader { final String _root; final Dio _dio; final FileSystem _fileSystem; - Completer? _inFlight; + final Map> _inFlightByLang = {}; LanguageDownloaderImpl({ required String root, @@ -38,12 +38,12 @@ class LanguageDownloaderImpl implements LanguageDownloader { @override Future download(String langCode) async { - // Serialize: wait for any in-flight download to finish - while (_inFlight != null) { - await _inFlight!.future; + // Serialize per language; different languages may download in parallel + while (_inFlightByLang.containsKey(langCode)) { + await _inFlightByLang[langCode]!.future; } final completer = Completer(); - _inFlight = completer; + _inFlightByLang[langCode] = completer; final dest = pathFor(langCode); final staging = '$dest.staging'; @@ -107,7 +107,7 @@ class LanguageDownloaderImpl implements LanguageDownloader { } rethrow; } finally { - _inFlight = null; + _inFlightByLang.remove(langCode); completer.complete(); } } diff --git a/lib/widgets/download_language_button.dart b/lib/widgets/download_language_button.dart index 17a3659..c0810ac 100644 --- a/lib/widgets/download_language_button.dart +++ b/lib/widgets/download_language_button.dart @@ -1,3 +1,4 @@ +import 'package:app4training/data/bulk_language_download.dart'; import 'package:app4training/data/globals.dart'; import 'package:app4training/design/theme.dart'; import 'package:app4training/l10n/generated/app_localizations.dart'; @@ -96,33 +97,28 @@ class _DownloadAllLanguagesButtonState }); // Get l10n now as we can't access context after async gap later final l10n = context.l10n; - int countDownloads = 0; - int countErrors = 0; - String lastLanguage = ''; - for (var languageCode in ref.read(availableLanguagesProvider)) { - if (!ref.watch(languageProvider(languageCode)).downloaded) { - if (await ref - .read(languageProvider(languageCode).notifier) - .download()) { - countDownloads++; - lastLanguage = languageCode; - } else { - countErrors++; - } - } - } - if (countDownloads > 0) { + final codesToDownload = [ + for (final languageCode in ref.read(availableLanguagesProvider)) + if (!ref.read(languageProvider(languageCode)).downloaded) + languageCode, + ]; + final result = await downloadLanguagesInParallel( + codesToDownload, + download: (code) => + ref.read(languageProvider(code).notifier).download(), + ); + if (result.successCount > 0) { // Show info message in snackbar - String text = (countDownloads == 1) - ? l10n - .downloadedLanguage(l10n.getLanguageName(lastLanguage)) - : l10n.downloadedNLanguages(countDownloads); + String text = (result.successCount == 1) + ? l10n.downloadedLanguage( + l10n.getLanguageName(result.lastSuccessCode)) + : l10n.downloadedNLanguages(result.successCount); final snackBar = SnackBar( content: Text(text), duration: snackBarQuickSuccessDuration); ref.watch(scaffoldMessengerProvider).showSnackBar(snackBar); } - if (countErrors > 0) { + if (result.errorCount > 0) { ref.watch(scaffoldMessengerProvider).showSnackBar(SnackBar( content: Text(l10n.downloadError), duration: snackBarErrorDuration)); diff --git a/lib/widgets/update_language_button.dart b/lib/widgets/update_language_button.dart index bc4a3fa..cf25d29 100644 --- a/lib/widgets/update_language_button.dart +++ b/lib/widgets/update_language_button.dart @@ -1,3 +1,4 @@ +import 'package:app4training/data/bulk_language_download.dart'; import 'package:app4training/data/globals.dart'; import 'package:app4training/l10n/generated/app_localizations.dart'; import 'package:app4training/l10n/l10n.dart'; @@ -83,31 +84,27 @@ class _UpdateAllLanguagesButtonState }); // Get l10n now as we can't access context after async gap later final l10n = context.l10n; - int countUpdates = 0; - int countErrors = 0; - String lastLanguage = ''; - for (var languageCode in ref.read(availableLanguagesProvider)) { - final status = ref.read(languageStatusProvider(languageCode)); - if (status.updatesAvailable && - ref.read(languageProvider(languageCode)).downloaded) { - if (await ref - .read(languageProvider(languageCode).notifier) - .download()) { - countUpdates++; - lastLanguage = languageCode; - } else { - countErrors++; - } - } - } - if (countUpdates > 0) { + final codesToUpdate = [ + for (final languageCode in ref.read(availableLanguagesProvider)) + if (ref.read(languageStatusProvider(languageCode)).updatesAvailable && + ref.read(languageProvider(languageCode)).downloaded) + languageCode, + ]; + final result = await downloadLanguagesInParallel( + codesToUpdate, + download: (code) => + ref.read(languageProvider(code).notifier).download(), + ); + if (result.successCount > 0) { // Show info message in snackbar - String text = (countUpdates == 1) - ? l10n.updatedLanguage(l10n.getLanguageName(lastLanguage)) - : l10n.updatedNLanguages(countUpdates, countErrors); + String text = (result.successCount == 1) + ? l10n.updatedLanguage( + l10n.getLanguageName(result.lastSuccessCode)) + : l10n.updatedNLanguages( + result.successCount, result.errorCount); final snackBar = SnackBar(content: Text(text)); ref.watch(scaffoldMessengerProvider).showSnackBar(snackBar); - } else if (countErrors > 0) { + } else if (result.errorCount > 0) { ref .watch(scaffoldMessengerProvider) .showSnackBar(SnackBar(content: Text(l10n.updateError))); diff --git a/test/bulk_language_download_test.dart b/test/bulk_language_download_test.dart new file mode 100644 index 0000000..76abc70 --- /dev/null +++ b/test/bulk_language_download_test.dart @@ -0,0 +1,111 @@ +import 'dart:async'; + +import 'package:app4training/data/bulk_language_download.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test('processes all language codes', () async { + final processed = []; + final result = await downloadLanguagesInParallel( + ['de', 'en', 'fr'], + maxConcurrent: 4, + download: (code) async { + processed.add(code); + return true; + }, + ); + + // Order is not guaranteed with a worker pool, only that all are processed. + expect(processed, unorderedEquals(['de', 'en', 'fr'])); + expect(result.successCount, 3); + expect(result.errorCount, 0); + expect(['de', 'en', 'fr'], contains(result.lastSuccessCode)); + }); + + test('aggregates successes and failures', () async { + final result = await downloadLanguagesInParallel( + ['de', 'en', 'fr'], + maxConcurrent: 4, + download: (code) async => code != 'en', + ); + + expect(result.successCount, 2); + expect(result.errorCount, 1); + expect(['de', 'fr'], contains(result.lastSuccessCode)); + }); + + test('counts a throwing download as an error without aborting the rest', + () async { + final result = await downloadLanguagesInParallel( + ['de', 'en', 'fr'], + maxConcurrent: 4, + download: (code) async { + if (code == 'en') throw Exception('boom'); + return true; + }, + ); + + expect(result.successCount, 2); + expect(result.errorCount, 1); + }); + + test('never exceeds maxConcurrent in flight', () async { + var inFlight = 0; + var maxObserved = 0; + final gate = Completer(); + + final future = downloadLanguagesInParallel( + List.generate(10, (i) => 'lang$i'), + maxConcurrent: 4, + download: (code) async { + inFlight++; + maxObserved = maxObserved < inFlight ? inFlight : maxObserved; + await gate.future; + inFlight--; + return true; + }, + ); + + while (maxObserved < 4) { + await Future.delayed(Duration.zero); + } + expect(maxObserved, 4); + + gate.complete(); + final result = await future; + expect(result.successCount, 10); + }); + + test('keeps the pool full instead of stalling on a slow download', () async { + final slowGate = Completer(); + final completed = []; + + final future = downloadLanguagesInParallel( + ['slow', 'a', 'b', 'c'], + maxConcurrent: 2, + download: (code) async { + if (code == 'slow') { + await slowGate.future; + } + completed.add(code); + return true; + }, + ); + + // While 'slow' is still blocked, the second worker should drain all the + // remaining fast downloads. A batched implementation would stall here. + while (completed.length < 3) { + await Future.delayed(Duration.zero); + } + expect(completed, containsAll(['a', 'b', 'c'])); + expect(completed, isNot(contains('slow'))); + + slowGate.complete(); + final result = await future; + expect(result.successCount, 4); + }); + + test('uses default maxConcurrent constant', () async { + expect(kMaxParallelLanguageDownloads, 4); + }); +} diff --git a/test/language_downloader_test.dart b/test/language_downloader_test.dart index b2d06f0..2b449da 100644 --- a/test/language_downloader_test.dart +++ b/test/language_downloader_test.dart @@ -1,4 +1,5 @@ import 'dart:typed_data'; +import 'dart:async'; import 'package:app4training/data/globals.dart'; import 'package:app4training/data/language_downloader.dart'; @@ -146,8 +147,11 @@ void main() { 'precious'); }); - test('Concurrent calls are serialized', () async { - final callOrder = []; + test('Different languages download in parallel', () async { + final deHtmlGate = Completer(); + final frHtmlGate = Completer(); + var deHtmlInFlight = false; + var frHtmlInFlight = false; final htmlZip1 = createTestZip({ '${Globals.getResourcesDir('de')}/file.txt': 'de-content', @@ -162,12 +166,12 @@ void main() { '${Globals.getPdfDir('fr')}/file.pdf': 'fr-pdf', }); - // Track call ordering via dio mocks when(() => dio.get>( Globals.getRemoteUrlHtml('de'), options: any(named: 'options'), )).thenAnswer((_) async { - callOrder.add('de-html-start'); + deHtmlInFlight = true; + await deHtmlGate.future; return Response( data: htmlZip1.toList(), statusCode: 200, @@ -177,19 +181,17 @@ void main() { when(() => dio.get>( Globals.getRemoteUrlPdf('de'), options: any(named: 'options'), - )).thenAnswer((_) async { - callOrder.add('de-pdf-start'); - return Response( - data: pdfZip1.toList(), - statusCode: 200, - requestOptions: RequestOptions(), - ); - }); + )).thenAnswer((_) async => Response( + data: pdfZip1.toList(), + statusCode: 200, + requestOptions: RequestOptions(), + )); when(() => dio.get>( Globals.getRemoteUrlHtml('fr'), options: any(named: 'options'), )).thenAnswer((_) async { - callOrder.add('fr-html-start'); + frHtmlInFlight = true; + await frHtmlGate.future; return Response( data: htmlZip2.toList(), statusCode: 200, @@ -199,24 +201,66 @@ void main() { when(() => dio.get>( Globals.getRemoteUrlPdf('fr'), options: any(named: 'options'), + )).thenAnswer((_) async => Response( + data: pdfZip2.toList(), + statusCode: 200, + requestOptions: RequestOptions(), + )); + + final f1 = downloader.download('de'); + await Future.delayed(Duration.zero); + final f2 = downloader.download('fr'); + await Future.delayed(Duration.zero); + + expect(deHtmlInFlight, isTrue); + expect(frHtmlInFlight, isTrue); + + deHtmlGate.complete(); + frHtmlGate.complete(); + await Future.wait([f1, f2]); + }); + + test('Same language downloads are serialized', () async { + var htmlCallCount = 0; + final gate = Completer(); + + final htmlZip = createTestZip({ + '${Globals.getResourcesDir('de')}/file.txt': 'de-content', + }); + final pdfZip = createTestZip({ + '${Globals.getPdfDir('de')}/file.pdf': 'de-pdf', + }); + + when(() => dio.get>( + Globals.getRemoteUrlHtml('de'), + options: any(named: 'options'), )).thenAnswer((_) async { - callOrder.add('fr-pdf-start'); + htmlCallCount++; + await gate.future; return Response( - data: pdfZip2.toList(), + data: htmlZip.toList(), statusCode: 200, requestOptions: RequestOptions(), ); }); + when(() => dio.get>( + Globals.getRemoteUrlPdf('de'), + options: any(named: 'options'), + )).thenAnswer((_) async => Response( + data: pdfZip.toList(), + statusCode: 200, + requestOptions: RequestOptions(), + )); - // Fire both without awaiting final f1 = downloader.download('de'); - final f2 = downloader.download('fr'); - await Future.wait([f1, f2]); + await Future.delayed(Duration.zero); + final f2 = downloader.download('de'); + await Future.delayed(Duration.zero); + expect(htmlCallCount, 1); - // de downloads must all complete before fr starts - final deLastIndex = callOrder.lastIndexOf('de-pdf-start'); - final frFirstIndex = callOrder.indexOf('fr-html-start'); - expect(deLastIndex, lessThan(frFirstIndex)); + gate.complete(); + await Future.wait([f1, f2]); + expect(htmlCallCount, 2); }); test('Crash recovery: pre-seeded staging dir is wiped by next download',