From 8b615bf7de37579823112a36157319a4a7272219 Mon Sep 17 00:00:00 2001 From: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> Date: Sun, 15 Sep 2024 20:02:09 +0530 Subject: [PATCH] refactor: sync --- .../android/app/src/main/AndroidManifest.xml | 2 +- .../com/alextran/immich/MainActivity.kt | 4 +- .../domain/interfaces/asset.interface.dart | 17 +- mobile-v2/lib/domain/models/asset.model.dart | 36 ++++- .../domain/repositories/asset.repository.dart | 23 +++ .../domain/repositories/store.repository.dart | 2 +- .../domain/services/asset_sync.service.dart | 149 ++++++++++++++++++ .../lib/domain/services/login.service.dart | 1 + .../lib/domain/services/sync.service.dart | 63 -------- mobile-v2/lib/immich_app.dart | 2 + .../components/grid/draggable_scrollbar.dart | 2 + .../grid/immich_asset_grid.state.dart | 2 +- .../components/image/immich_logo.widget.dart | 1 - .../adaptive_route_appbar.widget.dart | 1 - .../login/states/login_page.state.dart | 4 +- .../modules/settings/pages/settings.page.dart | 1 - mobile-v2/lib/service_locator.dart | 4 +- mobile-v2/lib/utils/collection_util.dart | 52 ++++++ .../utils/extensions/iterable.extension.dart | 4 + mobile-v2/lib/utils/isolate_helper.dart | 23 ++- 20 files changed, 307 insertions(+), 86 deletions(-) create mode 100644 mobile-v2/lib/domain/services/asset_sync.service.dart delete mode 100644 mobile-v2/lib/domain/services/sync.service.dart create mode 100644 mobile-v2/lib/utils/collection_util.dart create mode 100644 mobile-v2/lib/utils/extensions/iterable.extension.dart diff --git a/mobile-v2/android/app/src/main/AndroidManifest.xml b/mobile-v2/android/app/src/main/AndroidManifest.xml index 1df1b0843b..c6abaa296a 100644 --- a/mobile-v2/android/app/src/main/AndroidManifest.xml +++ b/mobile-v2/android/app/src/main/AndroidManifest.xml @@ -11,7 +11,7 @@ addAll(Iterable assets); + FutureOr addAll(Iterable assets); + + /// Removes assets with the [localIds] + FutureOr> fetchLocalAssetsForIds(List localIds); + + /// Removes assets with the [remoteIds] + FutureOr> fetchRemoteAssetsForIds(List remoteIds); + + /// Removes assets with the given [ids] + FutureOr deleteAssetsForIds(List ids); /// Removes all assets - Future clearAll(); + FutureOr clearAll(); /// Fetch assets from the [offset] with the [limit] - Future> fetchAssets({int? offset, int? limit}); + FutureOr> fetchAssets({int? offset, int? limit}); /// Streams assets as groups grouped by the group type passed Stream watchRenderList(); diff --git a/mobile-v2/lib/domain/models/asset.model.dart b/mobile-v2/lib/domain/models/asset.model.dart index 66f4c3a6e9..7efdb62dc9 100644 --- a/mobile-v2/lib/domain/models/asset.model.dart +++ b/mobile-v2/lib/domain/models/asset.model.dart @@ -1,3 +1,5 @@ +import 'package:flutter/material.dart'; +import 'package:immich_mobile/utils/collection_util.dart'; import 'package:immich_mobile/utils/extensions/string.extension.dart'; import 'package:openapi/api.dart'; @@ -70,8 +72,8 @@ class Asset { DateTime? createdTime, DateTime? modifiedTime, int? duration, - String? localId, - String? remoteId, + ValueGetter? localId, + ValueGetter? remoteId, String? livePhotoVideoId, }) { return Asset( @@ -84,12 +86,32 @@ class Asset { createdTime: createdTime ?? this.createdTime, modifiedTime: modifiedTime ?? this.modifiedTime, duration: duration ?? this.duration, - localId: localId ?? this.localId, - remoteId: remoteId ?? this.remoteId, + localId: localId != null ? localId() : this.localId, + remoteId: remoteId != null ? remoteId() : this.remoteId, livePhotoVideoId: livePhotoVideoId ?? this.livePhotoVideoId, ); } + Asset merge(Asset newAsset) { + if (newAsset.modifiedTime.isAfter(modifiedTime)) { + return newAsset.copyWith( + height: newAsset.height ?? height, + width: newAsset.width ?? width, + localId: () => newAsset.localId ?? localId, + remoteId: () => newAsset.remoteId ?? remoteId, + livePhotoVideoId: newAsset.livePhotoVideoId ?? livePhotoVideoId, + ); + } + + return copyWith( + height: height ?? newAsset.height, + width: width ?? newAsset.width, + localId: () => localId ?? newAsset.localId, + remoteId: () => remoteId ?? newAsset.remoteId, + livePhotoVideoId: livePhotoVideoId ?? newAsset.livePhotoVideoId, + ); + } + @override String toString() => """ { @@ -140,6 +162,12 @@ class Asset { remoteId.hashCode ^ livePhotoVideoId.hashCode; } + + static int compareByRemoteId(Asset a, Asset b) => + CollectionUtil.compareToNullable(a.remoteId, b.remoteId); + + static int compareByLocalId(Asset a, Asset b) => + CollectionUtil.compareToNullable(a.localId, b.localId); } AssetType _toAssetType(AssetTypeEnum type) => switch (type) { diff --git a/mobile-v2/lib/domain/repositories/asset.repository.dart b/mobile-v2/lib/domain/repositories/asset.repository.dart index 204b8c1cfc..f1e220a126 100644 --- a/mobile-v2/lib/domain/repositories/asset.repository.dart +++ b/mobile-v2/lib/domain/repositories/asset.repository.dart @@ -85,6 +85,29 @@ class RemoteAssetDriftRepository with LogContext implements IAssetRepository { .watch() .map((elements) => RenderList(elements: elements)); } + + @override + Future> fetchLocalAssetsForIds(List localIds) async { + final query = _db.asset.select() + ..where((row) => row.localId.isIn(localIds)) + ..orderBy([(asset) => OrderingTerm.asc(asset.localId)]); + + return (await query.get()).map(_toModel).toList(); + } + + @override + Future> fetchRemoteAssetsForIds(List remoteIds) async { + final query = _db.asset.select() + ..where((row) => row.remoteId.isIn(remoteIds)) + ..orderBy([(asset) => OrderingTerm.asc(asset.remoteId)]); + + return (await query.get()).map(_toModel).toList(); + } + + @override + FutureOr deleteAssetsForIds(List ids) async { + await _db.asset.deleteWhere((row) => row.id.isIn(ids)); + } } AssetCompanion _toEntity(Asset asset) { diff --git a/mobile-v2/lib/domain/repositories/store.repository.dart b/mobile-v2/lib/domain/repositories/store.repository.dart index 8896c5ac98..4d71516b97 100644 --- a/mobile-v2/lib/domain/repositories/store.repository.dart +++ b/mobile-v2/lib/domain/repositories/store.repository.dart @@ -75,7 +75,7 @@ class StoreDriftRepository with LogContext implements IStoreRepository { _ => null, } as U?; if (primitive != null) { - return key.converter.fromPrimitive(primitive); + return await key.converter.fromPrimitive(primitive); } return null; } diff --git a/mobile-v2/lib/domain/services/asset_sync.service.dart b/mobile-v2/lib/domain/services/asset_sync.service.dart new file mode 100644 index 0000000000..917301855e --- /dev/null +++ b/mobile-v2/lib/domain/services/asset_sync.service.dart @@ -0,0 +1,149 @@ +import 'dart:async'; + +import 'package:collection/collection.dart'; +import 'package:immich_mobile/domain/interfaces/asset.interface.dart'; +import 'package:immich_mobile/domain/models/asset.model.dart'; +import 'package:immich_mobile/domain/models/user.model.dart'; +import 'package:immich_mobile/service_locator.dart'; +import 'package:immich_mobile/utils/collection_util.dart'; +import 'package:immich_mobile/utils/constants/globals.dart'; +import 'package:immich_mobile/utils/immich_api_client.dart'; +import 'package:immich_mobile/utils/isolate_helper.dart'; +import 'package:immich_mobile/utils/mixins/log_context.mixin.dart'; +import 'package:logging/logging.dart'; +import 'package:openapi/api.dart'; + +class AssetSyncService with LogContext { + const AssetSyncService(); + + Future doFullRemoteSyncForUserDrift( + User user, { + DateTime? updatedUtil, + int? limit, + }) async { + return await IsolateHelper.run(() async { + try { + final logger = Logger("SyncService "); + final syncClient = di().getSyncApi(); + + final chunkSize = limit ?? kFullSyncChunkSize; + final updatedTill = updatedUtil ?? DateTime.now().toUtc(); + updatedUtil ??= DateTime.now().toUtc(); + String? lastAssetId; + + while (true) { + logger.info( + "Requesting more chunks from lastId - ${lastAssetId ?? ""}", + ); + + final assets = await syncClient.getFullSyncForUser(AssetFullSyncDto( + limit: chunkSize, + updatedUntil: updatedTill, + lastId: lastAssetId, + userId: user.id, + )); + if (assets == null) { + break; + } + + final assetsFromServer = + assets.map(Asset.remote).sorted(Asset.compareByRemoteId); + + final assetsInDb = + await di().fetchRemoteAssetsForIds( + assetsFromServer.map((a) => a.remoteId!).toList(), + ); + + await _syncAssetsToDbDrift( + assetsFromServer, + assetsInDb, + Asset.compareByRemoteId, + isRemoteSync: true, + ); + + lastAssetId = assets.lastOrNull?.id; + if (assets.length != chunkSize) break; + } + + return true; + } catch (e, s) { + log.severe("Error performing full sync for user - ${user.name}", e, s); + } + return false; + }); + } + + Future _syncAssetsToDbDrift( + List newAssets, + List existingAssets, + Comparator compare, { + bool? isRemoteSync, + }) async { + final (toAdd, toUpdate, assetsToRemove) = _diffAssets( + newAssets, + existingAssets, + compare: compare, + isRemoteSync: isRemoteSync, + ); + + final assetsToAdd = toAdd.followedBy(toUpdate); + + await di().addAll(assetsToAdd); + await di() + .deleteAssetsForIds(assetsToRemove.map((a) => a.id).toList()); + } + + /// Returns a triple (toAdd, toUpdate, toRemove) + (List, List, List) _diffAssets( + List newAssets, + List inDb, { + bool? isRemoteSync, + required Comparator compare, + }) { + // fast paths for trivial cases: reduces memory usage during initial sync etc. + if (newAssets.isEmpty && inDb.isEmpty) { + return const ([], [], []); + } else if (newAssets.isEmpty && isRemoteSync == null) { + // remove all from database + return (const [], const [], inDb); + } else if (inDb.isEmpty) { + // add all assets + return (newAssets, const [], const []); + } + + final List toAdd = []; + final List toUpdate = []; + final List toRemove = []; + CollectionUtil.diffSortedLists( + inDb, + newAssets, + compare: compare, + both: (Asset a, Asset b) { + if (a == b) { + toUpdate.add(a.merge(b)); + return true; + } + return false; + }, + // Only in DB (removed asset) + onlyFirst: (Asset a) { + // We are syncing remote assets, if asset only inDB, then it is removed from remote + if (isRemoteSync == true && a.isLocal) { + if (a.remoteId != null) { + toUpdate.add(a.copyWith(remoteId: () => null)); + } + // We are syncing local assets, mark the asset inDB as local only + } else if (isRemoteSync == false && a.isRemote) { + if (a.isLocal) { + toUpdate.add(a.copyWith(localId: () => null)); + } + } else { + toRemove.add(a); + } + }, + // Only in remote (new asset) + onlySecond: (Asset b) => toAdd.add(b), + ); + return (toAdd, toUpdate, toRemove); + } +} diff --git a/mobile-v2/lib/domain/services/login.service.dart b/mobile-v2/lib/domain/services/login.service.dart index db019d0051..acff9e1b49 100644 --- a/mobile-v2/lib/domain/services/login.service.dart +++ b/mobile-v2/lib/domain/services/login.service.dart @@ -132,6 +132,7 @@ class LoginService with LogContext { return false; } + ServiceLocator.registerCurrentUser(user); return true; } } diff --git a/mobile-v2/lib/domain/services/sync.service.dart b/mobile-v2/lib/domain/services/sync.service.dart deleted file mode 100644 index c7413f0e9a..0000000000 --- a/mobile-v2/lib/domain/services/sync.service.dart +++ /dev/null @@ -1,63 +0,0 @@ -import 'dart:async'; - -import 'package:immich_mobile/domain/interfaces/asset.interface.dart'; -import 'package:immich_mobile/domain/models/asset.model.dart'; -import 'package:immich_mobile/domain/models/user.model.dart'; -import 'package:immich_mobile/domain/repositories/database.repository.dart'; -import 'package:immich_mobile/service_locator.dart'; -import 'package:immich_mobile/utils/constants/globals.dart'; -import 'package:immich_mobile/utils/immich_api_client.dart'; -import 'package:immich_mobile/utils/isolate_helper.dart'; -import 'package:immich_mobile/utils/mixins/log_context.mixin.dart'; -import 'package:logging/logging.dart'; -import 'package:openapi/api.dart'; - -class SyncService with LogContext { - SyncService(); - - Future doFullSyncForUserDrift( - User user, { - DateTime? updatedUtil, - int? limit, - }) async { - return await IsolateHelper.run(() async { - try { - final logger = Logger("SyncService "); - final syncClient = di().getSyncApi(); - - final chunkSize = limit ?? kFullSyncChunkSize; - final updatedTill = updatedUtil ?? DateTime.now().toUtc(); - updatedUtil ??= DateTime.now().toUtc(); - String? lastAssetId; - - while (true) { - logger.info( - "Requesting more chunks from lastId - ${lastAssetId ?? ""}", - ); - - final assets = await syncClient.getFullSyncForUser(AssetFullSyncDto( - limit: chunkSize, - updatedUntil: updatedTill, - lastId: lastAssetId, - userId: user.id, - )); - if (assets == null) { - break; - } - - await di().addAll(assets.map(Asset.remote)); - - lastAssetId = assets.lastOrNull?.id; - if (assets.length != chunkSize) break; - } - - return true; - } catch (e, s) { - log.severe("Error performing full sync for user - ${user.name}", e, s); - } finally { - await di().close(); - } - return false; - }); - } -} diff --git a/mobile-v2/lib/immich_app.dart b/mobile-v2/lib/immich_app.dart index 4ac9b61d25..647b52d392 100644 --- a/mobile-v2/lib/immich_app.dart +++ b/mobile-v2/lib/immich_app.dart @@ -17,6 +17,8 @@ class ImApp extends StatefulWidget { } class _ImAppState extends State with WidgetsBindingObserver { + _ImAppState(); + @override Widget build(BuildContext context) { return TranslationProvider( diff --git a/mobile-v2/lib/presentation/components/grid/draggable_scrollbar.dart b/mobile-v2/lib/presentation/components/grid/draggable_scrollbar.dart index d57baa1cd5..662403b2e8 100644 --- a/mobile-v2/lib/presentation/components/grid/draggable_scrollbar.dart +++ b/mobile-v2/lib/presentation/components/grid/draggable_scrollbar.dart @@ -1,3 +1,5 @@ +// ignore_for_file: avoid-passing-self-as-argument + import 'dart:async'; import 'package:flutter/foundation.dart'; diff --git a/mobile-v2/lib/presentation/components/grid/immich_asset_grid.state.dart b/mobile-v2/lib/presentation/components/grid/immich_asset_grid.state.dart index 8bede41578..886c118a4f 100644 --- a/mobile-v2/lib/presentation/components/grid/immich_asset_grid.state.dart +++ b/mobile-v2/lib/presentation/components/grid/immich_asset_grid.state.dart @@ -8,7 +8,7 @@ import 'package:immich_mobile/domain/models/render_list.model.dart'; import 'package:immich_mobile/utils/constants/globals.dart'; typedef RenderListProvider = Stream Function(); -typedef RenderListAssetProvider = Future> Function({ +typedef RenderListAssetProvider = FutureOr> Function({ int? offset, int? limit, }); diff --git a/mobile-v2/lib/presentation/components/image/immich_logo.widget.dart b/mobile-v2/lib/presentation/components/image/immich_logo.widget.dart index bff9baf4e1..37e3675652 100644 --- a/mobile-v2/lib/presentation/components/image/immich_logo.widget.dart +++ b/mobile-v2/lib/presentation/components/image/immich_logo.widget.dart @@ -27,7 +27,6 @@ class ImLogo extends StatelessWidget { } } -// ignore: prefer-single-widget-per-file class ImLogoText extends StatelessWidget { const ImLogoText({ super.key, diff --git a/mobile-v2/lib/presentation/components/scaffold/adaptive_route_appbar.widget.dart b/mobile-v2/lib/presentation/components/scaffold/adaptive_route_appbar.widget.dart index 9186cc3209..bb6190b9ae 100644 --- a/mobile-v2/lib/presentation/components/scaffold/adaptive_route_appbar.widget.dart +++ b/mobile-v2/lib/presentation/components/scaffold/adaptive_route_appbar.widget.dart @@ -17,7 +17,6 @@ class ImAdaptiveRoutePrimaryAppBar extends StatelessWidget Size get preferredSize => const Size.fromHeight(kToolbarHeight); } -// ignore: prefer-single-widget-per-file class ImAdaptiveRouteSecondaryAppBar extends StatelessWidget implements PreferredSizeWidget { const ImAdaptiveRouteSecondaryAppBar({super.key}); diff --git a/mobile-v2/lib/presentation/modules/login/states/login_page.state.dart b/mobile-v2/lib/presentation/modules/login/states/login_page.state.dart index c2b8e8f7ac..ecf5fdc4f8 100644 --- a/mobile-v2/lib/presentation/modules/login/states/login_page.state.dart +++ b/mobile-v2/lib/presentation/modules/login/states/login_page.state.dart @@ -5,8 +5,8 @@ import 'package:immich_mobile/domain/interfaces/asset.interface.dart'; import 'package:immich_mobile/domain/interfaces/store.interface.dart'; import 'package:immich_mobile/domain/interfaces/user.interface.dart'; import 'package:immich_mobile/domain/models/store.model.dart'; +import 'package:immich_mobile/domain/services/asset_sync.service.dart'; import 'package:immich_mobile/domain/services/login.service.dart'; -import 'package:immich_mobile/domain/services/sync.service.dart'; import 'package:immich_mobile/domain/services/user.service.dart'; import 'package:immich_mobile/i18n/strings.g.dart'; import 'package:immich_mobile/presentation/modules/common/states/server_info/server_feature_config.state.dart'; @@ -139,7 +139,7 @@ class LoginPageCubit extends Cubit with LogContext { await di().add(user); // Remove and Sync assets in background await di().clearAll(); - unawaited(di().doFullSyncForUserDrift(user)); + unawaited(di().doFullRemoteSyncForUserDrift(user)); emit(state.copyWith( isValidationInProgress: false, diff --git a/mobile-v2/lib/presentation/modules/settings/pages/settings.page.dart b/mobile-v2/lib/presentation/modules/settings/pages/settings.page.dart index 0d63f8556c..834a678063 100644 --- a/mobile-v2/lib/presentation/modules/settings/pages/settings.page.dart +++ b/mobile-v2/lib/presentation/modules/settings/pages/settings.page.dart @@ -22,7 +22,6 @@ class SettingsWrapperPage extends StatelessWidget { } @RoutePage() -// ignore: prefer-single-widget-per-file class SettingsPage extends StatelessWidget { const SettingsPage({super.key}); diff --git a/mobile-v2/lib/service_locator.dart b/mobile-v2/lib/service_locator.dart index 5f64a38dee..e5a21de986 100644 --- a/mobile-v2/lib/service_locator.dart +++ b/mobile-v2/lib/service_locator.dart @@ -10,9 +10,9 @@ import 'package:immich_mobile/domain/repositories/log.repository.dart'; import 'package:immich_mobile/domain/repositories/store.repository.dart'; import 'package:immich_mobile/domain/repositories/user.repository.dart'; import 'package:immich_mobile/domain/services/app_setting.service.dart'; +import 'package:immich_mobile/domain/services/asset_sync.service.dart'; import 'package:immich_mobile/domain/services/login.service.dart'; import 'package:immich_mobile/domain/services/server_info.service.dart'; -import 'package:immich_mobile/domain/services/sync.service.dart'; import 'package:immich_mobile/domain/services/user.service.dart'; import 'package:immich_mobile/presentation/modules/common/states/current_user.state.dart'; import 'package:immich_mobile/presentation/modules/common/states/server_info/server_feature_config.state.dart'; @@ -92,7 +92,7 @@ class ServiceLocator { _registerFactory(() => ServerInfoService( di().getServerApi(), )); - _registerFactory(() => SyncService()); + _registerFactory(() => const AssetSyncService()); } static void registerPostGlobalStates() { diff --git a/mobile-v2/lib/utils/collection_util.dart b/mobile-v2/lib/utils/collection_util.dart new file mode 100644 index 0000000000..a62a08b00f --- /dev/null +++ b/mobile-v2/lib/utils/collection_util.dart @@ -0,0 +1,52 @@ +// ignore_for_file: avoid-unsafe-collection-methods + +class CollectionUtil { + const CollectionUtil(); + + static int compareToNullable(T? a, T? b) { + if (a == null) { + return 1; + } + if (b == null) { + return -1; + } + return a.compareTo(b); + } + + /// Find the difference between the two sorted lists [first] and [second] + /// Results are passed as callbacks back to the caller during the comparison + static bool diffSortedLists( + List first, + List second, { + required Comparator compare, + required bool Function(T a, T b) both, + required void Function(T a) onlyFirst, + required void Function(T b) onlySecond, + }) { + bool diff = false; + int i = 0, j = 0; + + for (; i < first.length && j < second.length;) { + final int order = compare(first[i], second[j]); + if (order == 0) { + diff |= both(first[i++], second[j++]); + } else if (order < 0) { + onlyFirst(first[i++]); + diff = true; + } else if (order > 0) { + onlySecond(second[j++]); + diff = true; + } + } + + diff |= i < first.length || j < second.length; + + for (; i < first.length; i++) { + onlyFirst(first[i]); + } + for (; j < second.length; j++) { + onlySecond(second[j]); + } + return diff; + } +} diff --git a/mobile-v2/lib/utils/extensions/iterable.extension.dart b/mobile-v2/lib/utils/extensions/iterable.extension.dart new file mode 100644 index 0000000000..cd8638af15 --- /dev/null +++ b/mobile-v2/lib/utils/extensions/iterable.extension.dart @@ -0,0 +1,4 @@ +extension SortIterable on Iterable { + Iterable sortedBy(Comparable Function(T k) key) => + toList()..sort((a, b) => key(a).compareTo(key(b))); +} diff --git a/mobile-v2/lib/utils/isolate_helper.dart b/mobile-v2/lib/utils/isolate_helper.dart index cda3477bb5..d8c11eddcb 100644 --- a/mobile-v2/lib/utils/isolate_helper.dart +++ b/mobile-v2/lib/utils/isolate_helper.dart @@ -16,13 +16,19 @@ class _ImApiClientData { const _ImApiClientData({required this.endpoint, required this.headersMap}); } +class InvalidIsolateUsageException implements Exception { + const InvalidIsolateUsageException(); + + @override + String toString() => + "IsolateHelper should only be used from the root isolate"; +} + // !! Should be used only from the root isolate class IsolateHelper { // Cache the ApiClient to reconstruct it later after inside the isolate late final _ImApiClientData? _clientData; - static RootIsolateToken get _rootToken => RootIsolateToken.instance!; - IsolateHelper(); void preIsolateHandling() { @@ -52,12 +58,21 @@ class IsolateHelper { } static Future run(FutureOr Function() computation) async { + final token = RootIsolateToken.instance; + if (token == null) { + throw const InvalidIsolateUsageException(); + } + final helper = IsolateHelper()..preIsolateHandling(); - final token = _rootToken; return await Isolate.run(() async { BackgroundIsolateBinaryMessenger.ensureInitialized(token); helper.postIsolateHandling(); - return await computation(); + try { + return await computation(); + } finally { + // Always close the new database connection on Isolate end + await di().close(); + } }); } }