review feedback, first service unit test

immich-app · jrasm91 · Sep 16, 2024 · Sep 15, 2024 · Sep 16, 2024 · Sep 16, 2024
commit df3233e3f3f51248bbfde61399ffc2ea2bee50b3
diff --git a/mobile/lib/interfaces/album.interface.dart b/mobile/lib/interfaces/album.interface.dart
@@ -3,7 +3,7 @@ import 'package:immich_mobile/entities/asset.entity.dart';
 import 'package:immich_mobile/entities/user.entity.dart';
 
 abstract interface class IAlbumRepository {
-  Future<int> countLocal();
+  Future<int> count({bool? local});
   Future<Album> create(Album album);
   Future<Album?> getById(int id);
   Future<Album?> getByName(
@@ -12,12 +12,10 @@ abstract interface class IAlbumRepository {
     bool? remote,
   });
   Future<Album> update(Album album);
-  Future<void> delete(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<DateTime?> getStartDate(Album album);
-  Future<DateTime?> getEndDate(Album album);
-  Future<DateTime?> getLastModified(Album album);
+  Future<Album> recalculateMetadata(Album album);
 }
diff --git a/mobile/lib/interfaces/asset.interface.dart b/mobile/lib/interfaces/asset.interface.dart
@@ -3,6 +3,6 @@ import 'package:immich_mobile/entities/asset.entity.dart';
 import 'package:immich_mobile/entities/user.entity.dart';
 
 abstract interface class IAssetRepository {
-  Future<List<Asset>> getByAlbumWithOwnerUnequal(Album album, User user);
+  Future<List<Asset>> getByAlbum(Album album, {User? notOwnedBy});
   Future<void> deleteById(List<int> ids);
 }
diff --git a/...lib/interfaces/backupalbum.interface.dart → mobile/lib/interfaces/backup.interface.dart b/...lib/interfaces/backupalbum.interface.dart → mobile/lib/interfaces/backup.interface.dart
@@ -1,5 +1,5 @@
 import 'package:immich_mobile/entities/backup_album.entity.dart';
 
-abstract interface class IBackupAlbumRepository {
+abstract interface class IBackupRepository {
   Future<List<String>> getIdsBySelection(BackupSelection backup);
 }
diff --git a/mobile/lib/repositories/album.repository.dart b/mobile/lib/repositories/album.repository.dart
@@ -17,7 +17,11 @@ class AlbumRepository implements IAlbumRepository {
   );
 
   @override
-  Future<int> countLocal() => _db.albums.where().localIdIsNotNull().count();
+  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) =>
@@ -42,8 +46,8 @@ class AlbumRepository implements IAlbumRepository {
       _db.writeTxn(() => _db.albums.store(album));
 
   @override
-  Future<void> delete(Album album) =>
-      _db.writeTxn(() => _db.albums.delete(album.id));
+  Future<void> delete(int albumId) =>
+      _db.writeTxn(() => _db.albums.delete(albumId));
 
   @override
   Future<List<Album>> getAll({bool? shared}) {
@@ -71,13 +75,11 @@ class AlbumRepository implements IAlbumRepository {
       _db.writeTxn(() => album.assets.update(unlink: assets));
 
   @override
-  Future<DateTime?> getStartDate(Album album) =>
-      album.assets.filter().fileCreatedAtProperty().min();
-  @override
-  Future<DateTime?> getEndDate(Album album) =>
-      album.assets.filter().fileCreatedAtProperty().max();
-
-  @override
-  Future<DateTime?> getLastModified(Album album) =>
-      album.assets.filter().updatedAtProperty().max();
+  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;
+  }
 }
diff --git a/mobile/lib/repositories/asset.repository.dart b/mobile/lib/repositories/asset.repository.dart
@@ -17,8 +17,13 @@ class AssetRepository implements IAssetRepository {
   );
 
   @override
-  Future<List<Asset>> getByAlbumWithOwnerUnequal(Album album, User user) =>
-      album.assets.filter().not().ownerIdEqualTo(user.isarId).findAll();
+  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) =>

diff --git a/.../repositories/backupalbum.repository.dart → ...e/lib/repositories/backup.repository.dart b/.../repositories/backupalbum.repository.dart → ...e/lib/repositories/backup.repository.dart
@@ -1,16 +1,16 @@
 import 'package:hooks_riverpod/hooks_riverpod.dart';
 import 'package:immich_mobile/entities/backup_album.entity.dart';
-import 'package:immich_mobile/interfaces/backupalbum.interface.dart';
+import 'package:immich_mobile/interfaces/backup.interface.dart';
 import 'package:immich_mobile/providers/db.provider.dart';
 import 'package:isar/isar.dart';
 
-final backupAlbumRepositoryProvider =
-    Provider((ref) => BackupAlbumRepository(ref.watch(dbProvider)));
+final backupRepositoryProvider =
+    Provider((ref) => BackupRepository(ref.watch(dbProvider)));
 
-class BackupAlbumRepository implements IBackupAlbumRepository {
+class BackupRepository implements IBackupRepository {
   final Isar _db;
 
-  BackupAlbumRepository(
+  BackupRepository(
     this._db,
   );
 

diff --git a/mobile/lib/services/album.service.dart b/mobile/lib/services/album.service.dart
@@ -7,7 +7,7 @@ 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/backupalbum.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';
@@ -18,7 +18,7 @@ import 'package:immich_mobile/entities/user.entity.dart';
 import 'package:immich_mobile/providers/api.provider.dart';
 import 'package:immich_mobile/repositories/album.repository.dart';
 import 'package:immich_mobile/repositories/asset.repository.dart';
-import 'package:immich_mobile/repositories/backupalbum.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';
@@ -35,7 +35,7 @@ final albumServiceProvider = Provider(
     ref.watch(albumRepositoryProvider),
     ref.watch(assetRepositoryProvider),
     ref.watch(userRepositoryProvider),
-    ref.watch(backupAlbumRepositoryProvider),
+    ref.watch(backupRepositoryProvider),
   ),
 );
 
@@ -46,7 +46,7 @@ class AlbumService {
   final IAlbumRepository _albumRepository;
   final IAssetRepository _assetRepository;
   final IUserRepository _userRepository;
-  final IBackupAlbumRepository _backupAlbumRepository;
+  final IBackupRepository _backupAlbumRepository;
   final Logger _log = Logger('AlbumService');
   Completer<bool> _localCompleter = Completer()..complete(false);
   Completer<bool> _remoteCompleter = Completer()..complete(false);
@@ -78,7 +78,7 @@ class AlbumService {
       final List<String> selectedIds = await _backupAlbumRepository
           .getIdsBySelection(BackupSelection.select);
       if (selectedIds.isEmpty) {
-        final numLocal = await _albumRepository.countLocal();
+        final numLocal = await _albumRepository.count(local: true);
         if (numLocal > 0) {
           _syncService.removeAllLocalAlbumsAndAssets();
         }
@@ -282,10 +282,7 @@ class AlbumService {
     if (album == null) return;
     await _albumRepository.addAssets(album, add);
     await _albumRepository.removeAssets(album, remove);
-    album.startDate = await _albumRepository.getStartDate(album);
-    album.endDate = await _albumRepository.getEndDate(album);
-    album.lastModifiedAssetTimestamp =
-        await _albumRepository.getLastModified(album);
+    await _albumRepository.recalculateMetadata(album);
     await _albumRepository.update(album);
   }
 
@@ -339,14 +336,14 @@ class AlbumService {
       }
       if (album.shared) {
         final foreignAssets =
-            await _assetRepository.getByAlbumWithOwnerUnequal(album, user);
-        await _albumRepository.delete(album);
+            await _assetRepository.getByAlbum(album, notOwnedBy: user);
+        await _albumRepository.delete(album.id);
 
         final List<Album> albums = await _albumRepository.getAll(shared: true);
         final List<Asset> existing = [];
         for (Album album in albums) {
           existing.addAll(
-            await _assetRepository.getByAlbumWithOwnerUnequal(album, user),
+            await _assetRepository.getByAlbum(album, notOwnedBy: user),
           );
         }
         final List<int> idsToRemove =
@@ -355,7 +352,7 @@ class AlbumService {
           await _assetRepository.deleteById(idsToRemove);
         }
       } else {
-        await _albumRepository.delete(album);
+        await _albumRepository.delete(album.id);
       }
       return true;
     } catch (e) {

diff --git a/mobile/lib/services/background.service.dart b/mobile/lib/services/background.service.dart
@@ -14,7 +14,7 @@ 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/backupalbum.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';
@@ -362,7 +362,7 @@ class BackgroundService {
     AlbumRepository albumRepository = AlbumRepository(db);
     AssetRepository assetRepository = AssetRepository(db);
     UserRepository userRepository = UserRepository(db);
-    BackupAlbumRepository backupAlbumRepository = BackupAlbumRepository(db);
+    BackupRepository backupAlbumRepository = BackupRepository(db);
     HashService hashService = HashService(db, this);
     SyncService syncSerive = SyncService(db, hashService);
     UserService userService =

diff --git a/mobile/test/repository.mocks.dart b/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 {}
diff --git a/mobile/test/service.mocks.dart b/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 {}
diff --git a/mobile/test/services/album.service.test.dart b/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());
+    });
+  });
+}