From 6b5defc27b750b76c3d8c9aa5881666fef633efd Mon Sep 17 00:00:00 2001
From: Mert <101130780+mertalev@users.noreply.github.com>
Date: Mon, 18 Nov 2024 10:26:23 -0500
Subject: [PATCH] fix(mobile): use sets in album refresh, concurrent futures
 (#14193)

* use sets in album sync, concurrent futures

* batch excluded asset IDs

* update test

* take advantage of sets in Recents check

* move log statement

* smaller diff
---
 mobile/lib/services/album.service.dart       | 56 +++++++++++---------
 mobile/test/services/album.service_test.dart |  1 +
 2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/mobile/lib/services/album.service.dart b/mobile/lib/services/album.service.dart
index 53a65e2869..1a23705916 100644
--- a/mobile/lib/services/album.service.dart
+++ b/mobile/lib/services/album.service.dart
@@ -76,10 +76,16 @@ class AlbumService {
     final Stopwatch sw = Stopwatch()..start();
     bool changes = false;
     try {
-      final List<String> excludedIds = await _backupAlbumRepository
-          .getIdsBySelection(BackupSelection.exclude);
-      final List<String> selectedIds = await _backupAlbumRepository
-          .getIdsBySelection(BackupSelection.select);
+      final (selectedIds, excludedIds, onDevice) = await (
+        _backupAlbumRepository
+            .getIdsBySelection(BackupSelection.select)
+            .then((value) => value.toSet()),
+        _backupAlbumRepository
+            .getIdsBySelection(BackupSelection.exclude)
+            .then((value) => value.toSet()),
+        _albumMediaRepository.getAll()
+      ).wait;
+      _log.info("Found ${onDevice.length} device albums");
       if (selectedIds.isEmpty) {
         final numLocal = await _albumRepository.count(local: true);
         if (numLocal > 0) {
@@ -87,8 +93,6 @@ class AlbumService {
         }
         return false;
       }
-      final List<Album> onDevice = await _albumMediaRepository.getAll();
-      _log.info("Found ${onDevice.length} device albums");
       Set<String>? excludedAssets;
       if (excludedIds.isNotEmpty) {
         if (Platform.isIOS) {
@@ -108,22 +112,19 @@ class AlbumService {
           "Ignoring ${excludedIds.length} excluded albums resulting in ${onDevice.length} device albums",
         );
       }
-      final hasAll = selectedIds
-          .map(
-            (id) => onDevice.firstWhereOrNull((album) => album.localId == id),
-          )
-          .whereNotNull()
-          .any((a) => a.isAll);
+
+      final allAlbum = onDevice.firstWhereOrNull((album) => album.isAll);
+      final hasAll = allAlbum != null && selectedIds.contains(allAlbum.localId);
       if (hasAll) {
         if (Platform.isAndroid) {
           // remove the virtual "Recent" album and keep and individual albums
           // on Android, the virtual "Recent" `lastModified` value is always null
-          onDevice.removeWhere((e) => e.isAll);
+          onDevice.removeWhere((album) => album.isAll);
           _log.info("'Recents' is selected, keeping all individual albums");
         }
       } else {
         // keep only the explicitly selected albums
-        onDevice.removeWhere((e) => !selectedIds.contains(e.localId));
+        onDevice.removeWhere((album) => !selectedIds.contains(album.localId));
         _log.info("'Recents' is not selected, keeping only selected albums");
       }
       changes =
@@ -138,15 +139,19 @@ class AlbumService {
 
   Future<Set<String>> _loadExcludedAssetIds(
     List<Album> albums,
-    List<String> excludedAlbumIds,
+    Set<String> excludedAlbumIds,
   ) async {
     final Set<String> result = HashSet<String>();
-    for (Album album in albums) {
-      if (excludedAlbumIds.contains(album.localId)) {
-        final assetIds =
-            await _albumMediaRepository.getAssetIds(album.localId!);
-        result.addAll(assetIds);
-      }
+    for (final batchAlbums in albums
+        .where((album) => excludedAlbumIds.contains(album.localId))
+        .slices(5)) {
+      await batchAlbums
+          .map(
+            (album) => _albumMediaRepository
+                .getAssetIds(album.localId!)
+                .then((assetIds) => result.addAll(assetIds)),
+          )
+          .wait;
     }
     return result;
   }
@@ -163,11 +168,10 @@ class AlbumService {
     bool changes = false;
     try {
       await _userService.refreshUsers();
-      final List<Album> sharedAlbum =
-          await _albumApiRepository.getAll(shared: true);
-
-      final List<Album> ownedAlbum =
-          await _albumApiRepository.getAll(shared: null);
+      final (sharedAlbum, ownedAlbum) = await (
+        _albumApiRepository.getAll(shared: true),
+        _albumApiRepository.getAll(shared: null)
+      ).wait;
 
       final albums = HashSet<Album>(
         equals: (a, b) => a.remoteId == b.remoteId,
diff --git a/mobile/test/services/album.service_test.dart b/mobile/test/services/album.service_test.dart
index 848d7cfad7..c0775a1c3e 100644
--- a/mobile/test/services/album.service_test.dart
+++ b/mobile/test/services/album.service_test.dart
@@ -54,6 +54,7 @@ void main() {
           .thenAnswer((_) async => []);
       when(() => backupRepository.getIdsBySelection(BackupSelection.select))
           .thenAnswer((_) async => []);
+      when(() => albumMediaRepository.getAll()).thenAnswer((_) async => []);
       when(() => albumRepository.count(local: true)).thenAnswer((_) async => 1);
       when(() => syncService.removeAllLocalAlbumsAndAssets())
           .thenAnswer((_) async => true);