review feedback

immich-app · Apr 26, 2024 · 71fc321 · 71fc321
1 parent ce19a49
commit 71fc321
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 39 deletions.
diff --git a/mobile/lib/shared/services/asset.service.dart b/mobile/lib/shared/services/asset.service.dart
@@ -50,10 +50,10 @@ class AssetService {
         .findAll();
     final Stopwatch sw = Stopwatch()..start();
     final bool changes = await _syncService.syncRemoteAssetsToDb(
-      users,
-      _getRemoteAssetChanges,
-      _getRemoteAssets,
-      _userService.getUsersFromServer,
+      users: users,
+      getChangedAssets: _getRemoteAssetChanges,
+      loadAssets: _getRemoteAssets,
+      refreshUsers: _userService.getUsersFromServer,
     );
     debugPrint("refreshRemoteAssets full took ${sw.elapsedMilliseconds}ms");
     return changes;
@@ -97,7 +97,8 @@ class AssetService {
       final List<Asset> allAssets = [];
       DateTime? lastCreationDate;
       String? lastId;
-      for (;;) {
+      // will break on error or once all assets are loaded
+      while (true) {
         final List<AssetResponseDto>? assets =
             await _apiService.syncApi.getAllForUserFullSync(
           chunkSize,

diff --git a/mobile/lib/shared/services/sync.service.dart b/mobile/lib/shared/services/sync.service.dart
@@ -40,15 +40,16 @@ class SyncService {
 
   /// Syncs remote assets owned by the logged-in user to the DB
   /// Returns `true` if there were any changes
-  Future<bool> syncRemoteAssetsToDb(
-    List<User> users,
-    Future<(List<Asset>? toUpsert, List<String>? toDelete)> Function(
+  Future<bool> syncRemoteAssetsToDb({
+    required List<User> users,
+    required Future<(List<Asset>? toUpsert, List<String>? toDelete)> Function(
       List<User> users,
       DateTime since,
     ) getChangedAssets,
-    FutureOr<List<Asset>?> Function(User user, DateTime until) loadAssets,
-    FutureOr<List<User>?> Function() refreshUsers,
-  ) =>
+    required FutureOr<List<Asset>?> Function(User user, DateTime until)
+        loadAssets,
+    required FutureOr<List<User>?> Function() refreshUsers,
+  }) =>
       _lock.run(
         () async =>
             await _syncRemoteAssetChanges(users, getChangedAssets) ??

diff --git a/mobile/test/modules/shared/sync_service_test.dart b/mobile/test/modules/shared/sync_service_test.dart
@@ -76,10 +76,10 @@ void main() {
       ];
       expect(db.assets.countSync(), 5);
       final bool c1 = await s.syncRemoteAssetsToDb(
-        [owner],
-        _failDiff,
-        (u, d) => remoteAssets,
-        () => [owner],
+        users: [owner],
+        getChangedAssets: _failDiff,
+        loadAssets: (u, d) => remoteAssets,
+        refreshUsers: () => [owner],
       );
       expect(c1, isFalse);
       expect(db.assets.countSync(), 5);
@@ -97,10 +97,10 @@ void main() {
       ];
       expect(db.assets.countSync(), 5);
       final bool c1 = await s.syncRemoteAssetsToDb(
-        [owner],
-        _failDiff,
-        (u, d) => remoteAssets,
-        () => [owner],
+        users: [owner],
+        getChangedAssets: _failDiff,
+        loadAssets: (u, d) => remoteAssets,
+        refreshUsers: () => [owner],
       );
       expect(c1, isTrue);
       expect(db.assets.countSync(), 7);
@@ -118,37 +118,37 @@ void main() {
       ];
       expect(db.assets.countSync(), 5);
       final bool c1 = await s.syncRemoteAssetsToDb(
-        [owner],
-        _failDiff,
-        (u, d) => remoteAssets,
-        () => [owner],
+        users: [owner],
+        getChangedAssets: _failDiff,
+        loadAssets: (u, d) => remoteAssets,
+        refreshUsers: () => [owner],
       );
       expect(c1, isTrue);
       expect(db.assets.countSync(), 8);
       final bool c2 = await s.syncRemoteAssetsToDb(
-        [owner],
-        _failDiff,
-        (u, d) => remoteAssets,
-        () => [owner],
+        users: [owner],
+        getChangedAssets: _failDiff,
+        loadAssets: (u, d) => remoteAssets,
+        refreshUsers: () => [owner],
       );
       expect(c2, isFalse);
       expect(db.assets.countSync(), 8);
       remoteAssets.removeAt(4);
       final bool c3 = await s.syncRemoteAssetsToDb(
-        [owner],
-        _failDiff,
-        (u, d) => remoteAssets,
-        () => [owner],
+        users: [owner],
+        getChangedAssets: _failDiff,
+        loadAssets: (u, d) => remoteAssets,
+        refreshUsers: () => [owner],
       );
       expect(c3, isTrue);
       expect(db.assets.countSync(), 7);
       remoteAssets.add(makeAsset(checksum: "k", remoteId: "2-1e"));
       remoteAssets.add(makeAsset(checksum: "l", remoteId: "2-2"));
       final bool c4 = await s.syncRemoteAssetsToDb(
-        [owner],
-        _failDiff,
-        (u, d) => remoteAssets,
-        () => [owner],
+        users: [owner],
+        getChangedAssets: _failDiff,
+        loadAssets: (u, d) => remoteAssets,
+        refreshUsers: () => [owner],
       );
       expect(c4, isTrue);
       expect(db.assets.countSync(), 9);
@@ -164,10 +164,10 @@ void main() {
       toUpsert[0].isFavorite = true;
       final List<String> toDelete = ["2-1", "1-1"];
       final bool c = await s.syncRemoteAssetsToDb(
-        [owner],
-        (user, since) async => (toUpsert, toDelete),
-        (user, date) => throw Exception(),
-        () => throw Exception(),
+        users: [owner],
+        getChangedAssets: (user, since) async => (toUpsert, toDelete),
+        loadAssets: (user, date) => throw Exception(),
+        refreshUsers: () => throw Exception(),
       );
       expect(c, isTrue);
       expect(db.assets.countSync(), 6);