refactor(mobile): repositories for album service (#12701)

* refactor(mobile): repositories for album service

* review feedback, first service unit test

mobile/lib/entities/album.entity.dart

@@ -164,12 +164,13 @@ class Album {
 }
 
 extension AssetsHelper on IsarCollection<Album> {
-  Future<void> store(Album a) async {
+  Future<Album> store(Album a) async {
     await put(a);
     await a.owner.save();
     await a.thumbnail.save();
     await a.sharedUsers.save();
     await a.assets.save();
+    return a;
   }
 }
 

mobile/lib/interfaces/album.interface.dart

@@ -0,0 +1,21 @@
+import 'package:immich_mobile/entities/album.entity.dart';
+import 'package:immich_mobile/entities/asset.entity.dart';
+import 'package:immich_mobile/entities/user.entity.dart';
+
+abstract interface class IAlbumRepository {
+  Future<int> count({bool? local});
+  Future<Album> create(Album album);
+  Future<Album?> getById(int id);
+  Future<Album?> getByName(
+    String name, {
+    bool? shared,
+    bool? remote,
+  });
+  Future<Album> update(Album album);
+  Future<void> delete(int albumId);
+  Future<List<Album>> getAll({bool? shared});
+  Future<void> removeUsers(Album album, List<User> users);
+  Future<void> addAssets(Album album, List<Asset> assets);
+  Future<void> removeAssets(Album album, List<Asset> assets);
+  Future<Album> recalculateMetadata(Album album);
+}

mobile/lib/interfaces/asset.interface.dart

@@ -0,0 +1,8 @@
+import 'package:immich_mobile/entities/album.entity.dart';
+import 'package:immich_mobile/entities/asset.entity.dart';
+import 'package:immich_mobile/entities/user.entity.dart';
+
+abstract interface class IAssetRepository {
+  Future<List<Asset>> getByAlbum(Album album, {User? notOwnedBy});
+  Future<void> deleteById(List<int> ids);
+}

mobile/lib/interfaces/backup.interface.dart

@@ -0,0 +1,5 @@
+import 'package:immich_mobile/entities/backup_album.entity.dart';
+
+abstract interface class IBackupRepository {
+  Future<List<String>> getIdsBySelection(BackupSelection backup);
+}

mobile/lib/interfaces/user.interface.dart

@@ -0,0 +1,5 @@
+import 'package:immich_mobile/entities/user.entity.dart';
+
+abstract interface class IUserRepository {
+  Future<List<User>> getByIds(List<String> ids);
+}

mobile/lib/repositories/album.repository.dart

@@ -0,0 +1,85 @@
+import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/entities/album.entity.dart';
+import 'package:immich_mobile/entities/asset.entity.dart';
+import 'package:immich_mobile/entities/user.entity.dart';
+import 'package:immich_mobile/interfaces/album.interface.dart';
+import 'package:immich_mobile/providers/db.provider.dart';
+import 'package:isar/isar.dart';
+
+final albumRepositoryProvider =
+    Provider((ref) => AlbumRepository(ref.watch(dbProvider)));
+
+class AlbumRepository implements IAlbumRepository {
+  final Isar _db;
+
+  AlbumRepository(
+    this._db,
+  );
+
+  @override
+  Future<int> count({bool? local}) {
+    if (local == true) return _db.albums.where().localIdIsNotNull().count();
+    if (local == false) return _db.albums.where().remoteIdIsNotNull().count();
+    return _db.albums.count();
+  }
+
+  @override
+  Future<Album> create(Album album) =>
+      _db.writeTxn(() => _db.albums.store(album));
+
+  @override
+  Future<Album?> getByName(String name, {bool? shared, bool? remote}) {
+    var query = _db.albums.filter().nameEqualTo(name);
+    if (shared != null) {
+      query = query.sharedEqualTo(shared);
+    }
+    if (remote == true) {
+      query = query.localIdIsNull();
+    } else if (remote == false) {
+      query = query.remoteIdIsNull();
+    }
+    return query.findFirst();
+  }
+
+  @override
+  Future<Album> update(Album album) =>
+      _db.writeTxn(() => _db.albums.store(album));
+
+  @override
+  Future<void> delete(int albumId) =>
+      _db.writeTxn(() => _db.albums.delete(albumId));
+
+  @override
+  Future<List<Album>> getAll({bool? shared}) {
+    final baseQuery = _db.albums.filter();
+    QueryBuilder<Album, Album, QAfterFilterCondition>? query;
+    if (shared != null) {
+      query = baseQuery.sharedEqualTo(true);
+    }
+    return query?.findAll() ?? _db.albums.where().findAll();
+  }
+
+  @override
+  Future<Album?> getById(int id) => _db.albums.get(id);
+
+  @override
+  Future<void> removeUsers(Album album, List<User> users) =>
+      _db.writeTxn(() => album.sharedUsers.update(unlink: users));
+
+  @override
+  Future<void> addAssets(Album album, List<Asset> assets) =>
+      _db.writeTxn(() => album.assets.update(link: assets));
+
+  @override
+  Future<void> removeAssets(Album album, List<Asset> assets) =>
+      _db.writeTxn(() => album.assets.update(unlink: assets));
+
+  @override
+  Future<Album> recalculateMetadata(Album album) async {
+    album.startDate = await album.assets.filter().fileCreatedAtProperty().min();
+    album.endDate = await album.assets.filter().fileCreatedAtProperty().max();
+    album.lastModifiedAssetTimestamp =
+        await album.assets.filter().updatedAtProperty().max();
+    return album;
+  }
+}

mobile/lib/repositories/asset.repository.dart

@@ -0,0 +1,31 @@
+import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/entities/album.entity.dart';
+import 'package:immich_mobile/entities/asset.entity.dart';
+import 'package:immich_mobile/entities/user.entity.dart';
+import 'package:immich_mobile/interfaces/asset.interface.dart';
+import 'package:immich_mobile/providers/db.provider.dart';
+import 'package:isar/isar.dart';
+
+final assetRepositoryProvider =
+    Provider((ref) => AssetRepository(ref.watch(dbProvider)));
+
+class AssetRepository implements IAssetRepository {
+  final Isar _db;
+
+  AssetRepository(
+    this._db,
+  );
+
+  @override
+  Future<List<Asset>> getByAlbum(Album album, {User? notOwnedBy}) {
+    var query = album.assets.filter();
+    if (notOwnedBy != null) {
+      query = query.not().ownerIdEqualTo(notOwnedBy.isarId);
+    }
+    return query.findAll();
+  }
+
+  @override
+  Future<void> deleteById(List<int> ids) =>
+      _db.writeTxn(() => _db.assets.deleteAll(ids));
+}

mobile/lib/repositories/backup.repository.dart

@@ -0,0 +1,20 @@
+import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/entities/backup_album.entity.dart';
+import 'package:immich_mobile/interfaces/backup.interface.dart';
+import 'package:immich_mobile/providers/db.provider.dart';
+import 'package:isar/isar.dart';
+
+final backupRepositoryProvider =
+    Provider((ref) => BackupRepository(ref.watch(dbProvider)));
+
+class BackupRepository implements IBackupRepository {
+  final Isar _db;
+
+  BackupRepository(
+    this._db,
+  );
+
+  @override
+  Future<List<String>> getIdsBySelection(BackupSelection backup) =>
+      _db.backupAlbums.filter().selectionEqualTo(backup).idProperty().findAll();
+}

mobile/lib/repositories/user.repository.dart

@@ -0,0 +1,20 @@
+import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/entities/user.entity.dart';
+import 'package:immich_mobile/interfaces/user.interface.dart';
+import 'package:immich_mobile/providers/db.provider.dart';
+import 'package:isar/isar.dart';
+
+final userRepositoryProvider =
+    Provider((ref) => UserRepository(ref.watch(dbProvider)));
+
+class UserRepository implements IUserRepository {
+  final Isar _db;
+
+  UserRepository(
+    this._db,
+  );
+
+  @override
+  Future<List<User>> getByIds(List<String> ids) async =>
+      (await _db.users.getAllById(ids)).cast();
+}

mobile/lib/services/album.service.dart

@@ -5,6 +5,10 @@ import 'dart:io';
 import 'package:collection/collection.dart';
 import 'package:flutter/foundation.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/interfaces/album.interface.dart';
+import 'package:immich_mobile/interfaces/asset.interface.dart';
+import 'package:immich_mobile/interfaces/backup.interface.dart';
+import 'package:immich_mobile/interfaces/user.interface.dart';
 import 'package:immich_mobile/models/albums/album_add_asset_response.model.dart';
 import 'package:immich_mobile/entities/backup_album.entity.dart';
 import 'package:immich_mobile/entities/album.entity.dart';
@@ -12,11 +16,13 @@ import 'package:immich_mobile/entities/asset.entity.dart';
 import 'package:immich_mobile/entities/store.entity.dart';
 import 'package:immich_mobile/entities/user.entity.dart';
 import 'package:immich_mobile/providers/api.provider.dart';
-import 'package:immich_mobile/providers/db.provider.dart';
+import 'package:immich_mobile/repositories/album.repository.dart';
+import 'package:immich_mobile/repositories/asset.repository.dart';
+import 'package:immich_mobile/repositories/backup.repository.dart';
+import 'package:immich_mobile/repositories/user.repository.dart';
 import 'package:immich_mobile/services/api.service.dart';
 import 'package:immich_mobile/services/sync.service.dart';
 import 'package:immich_mobile/services/user.service.dart';
-import 'package:isar/isar.dart';
 import 'package:logging/logging.dart';
 import 'package:openapi/api.dart';
 import 'package:photo_manager/photo_manager.dart';
@@ -26,7 +32,10 @@ final albumServiceProvider = Provider(
     ref.watch(apiServiceProvider),
     ref.watch(userServiceProvider),
     ref.watch(syncServiceProvider),
-    ref.watch(dbProvider),
+    ref.watch(albumRepositoryProvider),
+    ref.watch(assetRepositoryProvider),
+    ref.watch(userRepositoryProvider),
+    ref.watch(backupRepositoryProvider),
   ),
 );
 
@@ -34,7 +43,10 @@ class AlbumService {
   final ApiService _apiService;
   final UserService _userService;
   final SyncService _syncService;
-  final Isar _db;
+  final IAlbumRepository _albumRepository;
+  final IAssetRepository _assetRepository;
+  final IUserRepository _userRepository;
+  final IBackupRepository _backupAlbumRepository;
   final Logger _log = Logger('AlbumService');
   Completer<bool> _localCompleter = Completer()..complete(false);
   Completer<bool> _remoteCompleter = Completer()..complete(false);
@@ -43,16 +55,12 @@ class AlbumService {
     this._apiService,
     this._userService,
     this._syncService,
-    this._db,
+    this._albumRepository,
+    this._assetRepository,
+    this._userRepository,
+    this._backupAlbumRepository,
   );
 
-  QueryBuilder<BackupAlbum, BackupAlbum, QAfterFilterCondition>
-      selectedAlbumsQuery() =>
-          _db.backupAlbums.filter().selectionEqualTo(BackupSelection.select);
-  QueryBuilder<BackupAlbum, BackupAlbum, QAfterFilterCondition>
-      excludedAlbumsQuery() =>
-          _db.backupAlbums.filter().selectionEqualTo(BackupSelection.exclude);
-
   /// Checks all selected device albums for changes of albums and their assets
   /// Updates the local database and returns `true` if there were any changes
   Future<bool> refreshDeviceAlbums() async {
@@ -65,12 +73,12 @@ class AlbumService {
     final Stopwatch sw = Stopwatch()..start();
     bool changes = false;
     try {
-      final List<String> excludedIds =
+      final List<String> excludedIds = await _backupAlbumRepository
-          await excludedAlbumsQuery().idProperty().findAll();
+          .getIdsBySelection(BackupSelection.exclude);
-      final List<String> selectedIds =
+      final List<String> selectedIds = await _backupAlbumRepository
-          await selectedAlbumsQuery().idProperty().findAll();
+          .getIdsBySelection(BackupSelection.select);
       if (selectedIds.isEmpty) {
-        final numLocal = await _db.albums.where().localIdIsNotNull().count();
+        final numLocal = await _albumRepository.count(local: true);
         if (numLocal > 0) {
           _syncService.removeAllLocalAlbumsAndAssets();
         }
@@ -194,8 +202,8 @@ class AlbumService {
         ),
       );
       if (remote != null) {
-        Album album = await Album.remote(remote);
+        final Album album = await Album.remote(remote);
-        await _db.writeTxn(() => _db.albums.store(album));
+        await _albumRepository.create(album);
         return album;
       }
     } catch (e) {
@@ -212,8 +220,7 @@ class AlbumService {
     for (int round = 0;; round++) {
       final proposedName = "$baseName${round == 0 ? "" : " ($round)"}";
 
-      if (null ==
+      if (null == await _albumRepository.getByName(proposedName)) {
-          await _db.albums.filter().nameEqualTo(proposedName).findFirst()) {
         return proposedName;
       }
     }
@@ -268,20 +275,15 @@ class AlbumService {
 
   Future<void> _updateAssets(
     int albumId, {
-    Iterable<Asset> add = const [],
+    List<Asset> add = const [],
-    Iterable<Asset> remove = const [],
+    List<Asset> remove = const [],
-  }) {
+  }) async {
-    return _db.writeTxn(() async {
+    final album = await _albumRepository.getById(albumId);
-      final album = await _db.albums.get(albumId);
     if (album == null) return;
-      await album.assets.update(link: add, unlink: remove);
+    await _albumRepository.addAssets(album, add);
-      album.startDate =
+    await _albumRepository.removeAssets(album, remove);
-          await album.assets.filter().fileCreatedAtProperty().min();
+    await _albumRepository.recalculateMetadata(album);
-      album.endDate = await album.assets.filter().fileCreatedAtProperty().max();
+    await _albumRepository.update(album);
-      album.lastModifiedAssetTimestamp =
-          await album.assets.filter().updatedAtProperty().max();
-      await _db.albums.put(album);
-    });
   }
 
   Future<bool> addAdditionalUserToAlbum(
@@ -298,13 +300,9 @@ class AlbumService {
         AddUsersDto(albumUsers: albumUsers),
       );
       if (result != null) {
-        album.sharedUsers
+        album.sharedUsers.addAll(await _userRepository.getByIds(sharedUserIds));
-            .addAll((await _db.users.getAllById(sharedUserIds)).cast());
         album.shared = result.shared;
-        await _db.writeTxn(() async {
+        await _albumRepository.update(album);
-          await _db.albums.put(album);
-          await album.sharedUsers.save();
-        });
         return true;
       }
     } catch (e) {
@@ -321,7 +319,7 @@ class AlbumService {
       );
       if (result != null) {
         album.activityEnabled = enabled;
-        await _db.writeTxn(() => _db.albums.put(album));
+        await _albumRepository.update(album);
         return true;
       }
     } catch (e) {
@@ -332,29 +330,29 @@ class AlbumService {
 
   Future<bool> deleteAlbum(Album album) async {
     try {
-      final userId = Store.get(StoreKey.currentUser).isarId;
+      final user = Store.get(StoreKey.currentUser);
-      if (album.owner.value?.isarId == userId) {
+      if (album.owner.value?.isarId == user.isarId) {
         await _apiService.albumsApi.deleteAlbum(album.remoteId!);
       }
       if (album.shared) {
         final foreignAssets =
-            await album.assets.filter().not().ownerIdEqualTo(userId).findAll();
+            await _assetRepository.getByAlbum(album, notOwnedBy: user);
-        await _db.writeTxn(() => _db.albums.delete(album.id));
+        await _albumRepository.delete(album.id);
-        final List<Album> albums =
+
-            await _db.albums.filter().sharedEqualTo(true).findAll();
+        final List<Album> albums = await _albumRepository.getAll(shared: true);
         final List<Asset> existing = [];
-        for (Album a in albums) {
+        for (Album album in albums) {
           existing.addAll(
-            await a.assets.filter().not().ownerIdEqualTo(userId).findAll(),
+            await _assetRepository.getByAlbum(album, notOwnedBy: user),
           );
         }
         final List<int> idsToRemove =
             _syncService.sharedAssetsToRemove(foreignAssets, existing);
         if (idsToRemove.isNotEmpty) {
-          await _db.writeTxn(() => _db.assets.deleteAll(idsToRemove));
+          await _assetRepository.deleteById(idsToRemove);
         }
       } else {
-        await _db.writeTxn(() => _db.albums.delete(album.id));
+        await _albumRepository.delete(album.id);
       }
       return true;
     } catch (e) {
@@ -390,7 +388,7 @@ class AlbumService {
             : response
                 .where((e) => e.success)
                 .map((e) => assets.firstWhere((a) => a.remoteId == e.id));
-        await _updateAssets(album.id, remove: toRemove);
+        await _updateAssets(album.id, remove: toRemove.toList());
         return true;
       }
     } catch (e) {
@@ -410,12 +408,10 @@ class AlbumService {
       );
 
       album.sharedUsers.remove(user);
-      await _db.writeTxn(() async {
+      await _albumRepository.removeUsers(album, [user]);
-        await album.sharedUsers.update(unlink: [user]);
+      final a = await _albumRepository.getById(album.id);
-        final a = await _db.albums.get(album.id);
       // trigger watcher
-        await _db.albums.put(a!);
+      await _albumRepository.update(a!);
-      });
 
       return true;
     } catch (e) {
@@ -436,7 +432,7 @@ class AlbumService {
         ),
       );
       album.name = newAlbumTitle;
-      await _db.writeTxn(() => _db.albums.put(album));
+      await _albumRepository.update(album);
 
       return true;
     } catch (e) {
@@ -445,14 +441,8 @@ class AlbumService {
     }
   }
 
-  Future<Album?> getAlbumByName(String name, bool remoteOnly) async {
+  Future<Album?> getAlbumByName(String name, bool remoteOnly) =>
-    return _db.albums
+      _albumRepository.getByName(name, remote: remoteOnly ? true : null);
-        .filter()
-        .optional(remoteOnly, (q) => q.localIdIsNull())
-        .nameEqualTo(name)
-        .sharedEqualTo(false)
-        .findFirst();
-  }
 
   ///
   /// Add the uploaded asset to the selected albums

mobile/lib/services/background.service.dart

@@ -12,6 +12,10 @@ import 'package:hooks_riverpod/hooks_riverpod.dart';
 import 'package:immich_mobile/main.dart';
 import 'package:immich_mobile/models/backup/backup_candidate.model.dart';
 import 'package:immich_mobile/models/backup/success_upload_asset.model.dart';
+import 'package:immich_mobile/repositories/album.repository.dart';
+import 'package:immich_mobile/repositories/asset.repository.dart';
+import 'package:immich_mobile/repositories/backup.repository.dart';
+import 'package:immich_mobile/repositories/user.repository.dart';
 import 'package:immich_mobile/services/album.service.dart';
 import 'package:immich_mobile/services/hash.service.dart';
 import 'package:immich_mobile/services/localization.service.dart';
@@ -355,12 +359,23 @@ class BackgroundService {
     AppSettingsService settingService = AppSettingsService();
     AppSettingsService settingsService = AppSettingsService();
     PartnerService partnerService = PartnerService(apiService, db);
+    AlbumRepository albumRepository = AlbumRepository(db);
+    AssetRepository assetRepository = AssetRepository(db);
+    UserRepository userRepository = UserRepository(db);
+    BackupRepository backupAlbumRepository = BackupRepository(db);
     HashService hashService = HashService(db, this);
     SyncService syncSerive = SyncService(db, hashService);
     UserService userService =
         UserService(apiService, db, syncSerive, partnerService);
-    AlbumService albumService =
+    AlbumService albumService = AlbumService(
-        AlbumService(apiService, userService, syncSerive, db);
+      apiService,
+      userService,
+      syncSerive,
+      albumRepository,
+      assetRepository,
+      userRepository,
+      backupAlbumRepository,
+    );
     BackupService backupService =
         BackupService(apiService, db, settingService, albumService);
 

mobile/test/repository.mocks.dart

@@ -0,0 +1,13 @@
+import 'package:immich_mobile/interfaces/album.interface.dart';
+import 'package:immich_mobile/interfaces/asset.interface.dart';
+import 'package:immich_mobile/interfaces/backup.interface.dart';
+import 'package:immich_mobile/interfaces/user.interface.dart';
+import 'package:mocktail/mocktail.dart';
+
+class MockAlbumRepository extends Mock implements IAlbumRepository {}
+
+class MockAssetRepository extends Mock implements IAssetRepository {}
+
+class MockUserRepository extends Mock implements IUserRepository {}
+
+class MockBackupRepository extends Mock implements IBackupRepository {}

mobile/test/service.mocks.dart

@@ -0,0 +1,10 @@
+import 'package:immich_mobile/services/api.service.dart';
+import 'package:immich_mobile/services/sync.service.dart';
+import 'package:immich_mobile/services/user.service.dart';
+import 'package:mocktail/mocktail.dart';
+
+class MockApiService extends Mock implements ApiService {}
+
+class MockUserService extends Mock implements UserService {}
+
+class MockSyncService extends Mock implements SyncService {}

mobile/test/services/album.service.test.dart

@@ -0,0 +1,52 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:immich_mobile/entities/backup_album.entity.dart';
+import 'package:immich_mobile/services/album.service.dart';
+import 'package:mocktail/mocktail.dart';
+import '../repository.mocks.dart';
+import '../service.mocks.dart';
+
+void main() {
+  late AlbumService sut;
+  late MockApiService apiService;
+  late MockUserService userService;
+  late MockSyncService syncService;
+  late MockAlbumRepository albumRepository;
+  late MockAssetRepository assetRepository;
+  late MockUserRepository userRepository;
+  late MockBackupRepository backupRepository;
+
+  setUp(() {
+    apiService = MockApiService();
+    userService = MockUserService();
+    syncService = MockSyncService();
+    albumRepository = MockAlbumRepository();
+    assetRepository = MockAssetRepository();
+    userRepository = MockUserRepository();
+    backupRepository = MockBackupRepository();
+
+    sut = AlbumService(
+      apiService,
+      userService,
+      syncService,
+      albumRepository,
+      assetRepository,
+      userRepository,
+      backupRepository,
+    );
+  });
+
+  group('refreshDeviceAlbums', () {
+    test('empty selection with one album in db', () async {
+      when(() => backupRepository.getIdsBySelection(BackupSelection.exclude))
+          .thenAnswer((_) async => []);
+      when(() => backupRepository.getIdsBySelection(BackupSelection.select))
+          .thenAnswer((_) async => []);
+      when(() => albumRepository.count(local: true)).thenAnswer((_) async => 1);
+      when(() => syncService.removeAllLocalAlbumsAndAssets())
+          .thenAnswer((_) async => true);
+      final result = await sut.refreshDeviceAlbums();
+      expect(result, false);
+      verify(() => syncService.removeAllLocalAlbumsAndAssets());
+    });
+  });
+}