From 6874c07dbe0f6443d43e8a014fd18cfa6783c7c5 Mon Sep 17 00:00:00 2001 From: Adam Gastineau Date: Fri, 11 Sep 2026 10:56:00 -0700 Subject: [PATCH] chore(mobile): static Store to unify and streamline reactivity (#31099) * chore(mobile): static Store to unify and streamline reactivity * Add tests of caching and new stores * Don't instantiate PeopleDbRepository * Slight DB rename and fix throwing store methods --- mobile/lib/data/db/main/dao/person.dart | 4 +- mobile/lib/data/db/main/database.dart | 2 +- .../server/activity.dart} | 12 +- mobile/lib/data/store.dart | 9 + mobile/lib/data/store/activity.dart | 90 ++++++++++ mobile/lib/data/store/person.dart | 63 +++++++ mobile/lib/data/store/util/cache.dart | 164 ++++++++++++++++++ .../lib/domain/services/people.service.dart | 35 ---- .../lib/models/activities/activity.model.dart | 1 + .../presentation/pages/activities.page.dart | 12 +- .../lib/presentation/pages/library.page.dart | 4 +- .../pages/people_collection.page.dart | 4 +- .../like_activity_action_button.widget.dart | 18 +- .../asset_details/people_details.widget.dart | 7 +- .../viewer_top_app_bar.widget.dart | 4 +- .../person_edit_birthday_modal.widget.dart | 4 +- .../people/person_edit_name_modal.widget.dart | 4 +- mobile/lib/providers/activity.provider.dart | 81 --------- .../providers/activity_service.provider.dart | 13 -- .../infrastructure/people.provider.dart | 21 --- mobile/lib/services/activity.service.dart | 66 ------- mobile/lib/services/deep_link.service.dart | 14 +- mobile/lib/utils/provider_utils.dart | 2 +- .../widgets/activities/comment_bubble.dart | 36 ++-- .../search/search_filter/people_picker.dart | 6 +- .../test/data/store/activity_store_test.dart | 86 +++++++++ mobile/test/data/store/person_store_test.dart | 97 +++++++++++ mobile/test/data/store/util/cache_test.dart | 146 ++++++++++++++++ .../repositories/people_repository_test.dart | 4 +- .../test/services/deep_link_service_test.dart | 11 +- 30 files changed, 734 insertions(+), 286 deletions(-) rename mobile/lib/{repositories/activity_api.repository.dart => data/server/activity.dart} (77%) create mode 100644 mobile/lib/data/store.dart create mode 100644 mobile/lib/data/store/activity.dart create mode 100644 mobile/lib/data/store/person.dart create mode 100644 mobile/lib/data/store/util/cache.dart delete mode 100644 mobile/lib/domain/services/people.service.dart delete mode 100644 mobile/lib/providers/activity.provider.dart delete mode 100644 mobile/lib/providers/activity_service.provider.dart delete mode 100644 mobile/lib/providers/infrastructure/people.provider.dart delete mode 100644 mobile/lib/services/activity.service.dart create mode 100644 mobile/test/data/store/activity_store_test.dart create mode 100644 mobile/test/data/store/person_store_test.dart create mode 100644 mobile/test/data/store/util/cache_test.dart diff --git a/mobile/lib/data/db/main/dao/person.dart b/mobile/lib/data/db/main/dao/person.dart index d5f378274..cddf8bfd8 100644 --- a/mobile/lib/data/db/main/dao/person.dart +++ b/mobile/lib/data/db/main/dao/person.dart @@ -6,8 +6,8 @@ import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/domain/models/person.model.dart'; @DriftAccessor() -class PeopleRepository extends DatabaseAccessor with $PeopleRepositoryMixin { - PeopleRepository(super.attachedDatabase); +class PeopleDatabaseRepository extends DatabaseAccessor with $PeopleDatabaseRepositoryMixin { + PeopleDatabaseRepository(super.attachedDatabase); Drift get _db => attachedDatabase; diff --git a/mobile/lib/data/db/main/database.dart b/mobile/lib/data/db/main/database.dart index bebac570b..2868eede3 100644 --- a/mobile/lib/data/db/main/database.dart +++ b/mobile/lib/data/db/main/database.dart @@ -96,7 +96,7 @@ import 'package:sqlite_async/sqlite_async.dart'; MemoryRepository, OcrRepository, PartnerRepository, - PeopleRepository, + PeopleDatabaseRepository, RemoteAlbumRepository, RemoteAssetRepository, RemoteExifRepository, diff --git a/mobile/lib/repositories/activity_api.repository.dart b/mobile/lib/data/server/activity.dart similarity index 77% rename from mobile/lib/repositories/activity_api.repository.dart rename to mobile/lib/data/server/activity.dart index 5097a3934..122b54164 100644 --- a/mobile/lib/repositories/activity_api.repository.dart +++ b/mobile/lib/data/server/activity.dart @@ -1,8 +1,8 @@ import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/server/api_repository.dart'; import 'package:immich_mobile/infrastructure/utils/user.converter.dart'; import 'package:immich_mobile/models/activities/activity.model.dart'; import 'package:immich_mobile/providers/api.provider.dart'; -import 'package:immich_mobile/repositories/api.repository.dart'; import 'package:openapi/api.dart'; final activityApiRepositoryProvider = Provider( @@ -12,11 +12,11 @@ final activityApiRepositoryProvider = Provider( class ActivityApiRepository extends ApiRepository { final ActivitiesApi _api; - ActivityApiRepository(this._api); + const ActivityApiRepository(this._api); Future> getAll(String albumId, {String? assetId}) async { final response = await checkNull(_api.getActivities(albumId, assetId: assetId)); - return response.map(_toActivity).toList(); + return response.map((dto) => _toActivity(dto, albumId)).toList(); } Future create(String albumId, ActivityType type, {String? assetId, String? comment}) async { @@ -27,15 +27,17 @@ class ActivityApiRepository extends ApiRepository { comment: comment == null ? const Optional.absent() : Optional.present(comment), ); final response = await checkNull(_api.createActivity(dto)); - return _toActivity(response); + return _toActivity(response, albumId); } Future delete(String id) { + // TODO(agg23): I think this is a bug; `checkNull` will always throw here as the response is void (null) return checkNull(_api.deleteActivity(id)); } - static Activity _toActivity(ActivityResponseDto dto) => Activity( + static Activity _toActivity(ActivityResponseDto dto, String albumId) => Activity( id: dto.id, + albumId: albumId, createdAt: dto.createdAt, type: dto.type == ReactionType.comment ? ActivityType.comment : ActivityType.like, user: UserConverter.fromSimpleUserDto(dto.user), diff --git a/mobile/lib/data/store.dart b/mobile/lib/data/store.dart new file mode 100644 index 000000000..7eeb6b7ec --- /dev/null +++ b/mobile/lib/data/store.dart @@ -0,0 +1,9 @@ +import 'package:immich_mobile/data/store/activity.dart'; +import 'package:immich_mobile/data/store/person.dart'; + +/// Global data layer, providing access to Drift and HTTP APIs, scoped by entity +abstract final class Store { + static final people = PersonStore.instance; + + static final activity = ActivityStore.instance; +} diff --git a/mobile/lib/data/store/activity.dart b/mobile/lib/data/store/activity.dart new file mode 100644 index 000000000..c2ea5bad9 --- /dev/null +++ b/mobile/lib/data/store/activity.dart @@ -0,0 +1,90 @@ +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/server/activity.dart'; +import 'package:immich_mobile/data/server/errors.dart'; +import 'package:immich_mobile/data/store/util/cache.dart'; +import 'package:immich_mobile/models/activities/activity.model.dart'; +import 'package:logging/logging.dart'; + +final _log = Logger("ActivityStore"); + +/// The `albumId` and optional `assetId` identifying an activity list +typedef ActivityScope = (String albumId, String? assetId); + +/// Activities (comments and likes) on shared albums and their assets +/// +/// State is fetched over HTTP and mutated and cached in memory +extension type const ActivityStore._(Provider _provider) implements Provider { + /// Internal: access through `Store.activity` + static final ActivityStore instance = ActivityStore._(Provider((ref) => ActivityMutations._(ref))); + + /// All activities for an album specified by [albumId], or all activities for a specific asset within that album + /// + /// **NOTE:** This is currently reactive only to in memory mutation, not live HTTP changes + CacheListQuery list(String albumId, {String? assetId}) => _cache.family((albumId, assetId)); +} + +final _cache = _ActivityCache(); + +class _ActivityCache extends StoreCache { + @override + Future> fetch(Ref ref, ActivityScope scope) async { + try { + return await ref.watch(activityApiRepositoryProvider).getAll(scope.$1, assetId: scope.$2); + } catch (error, stack) { + _log.severe("Failed to get all activities for album ${scope.$1}", error, stack); + return const []; + } + } + + @override + Object identity(Activity item) => item.id; + + @override + bool shouldContain(ActivityScope scope, Activity item) => + scope.$1 == item.albumId && (scope.$2 == null || scope.$2 == item.assetId); +} + +class ActivityMutations extends CachedStoreMutations { + ActivityMutations._(Ref ref) : super(ref, _cache); + + /// Add a comment to an album or asset. Providing [assetId] will add to the corresponding asset, otherwise the comment will be added to the album + Future addComment(String albumId, String comment, {String? assetId}) async { + try { + final activity = await read( + activityApiRepositoryProvider, + ).create(albumId, ActivityType.comment, assetId: assetId, comment: comment); + cacheUpsert(activity); + return activity; + } catch (error, stack) { + _log.severe("Failed to create comment for album $albumId", error, stack); + rethrow; + } + } + + /// Add a like to an album or asset. Providing [assetId] will add to the corresponding asset, otherwise the like will be added to the album + Future addLike(String albumId, {String? assetId}) async { + try { + final activity = await read(activityApiRepositoryProvider).create(albumId, ActivityType.like, assetId: assetId); + cacheUpsert(activity); + return activity; + } catch (error, stack) { + _log.severe("Failed to create like for album $albumId", error, stack); + rethrow; + } + } + + /// Remove [activity] from its album + Future remove(Activity activity) async { + try { + await read(activityApiRepositoryProvider).delete(activity.id); + } on NoResponseDtoError { + // TODO(agg23): This error should not be thrown at all + } catch (error, stack) { + _log.severe("Failed to delete activity", error, stack); + rethrow; + } + + // Only apply on success (including the broken NoResponseDtoError) + cacheRemove(activity.id); + } +} diff --git a/mobile/lib/data/store/person.dart b/mobile/lib/data/store/person.dart new file mode 100644 index 000000000..dcd76dcc4 --- /dev/null +++ b/mobile/lib/data/store/person.dart @@ -0,0 +1,63 @@ +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/server/person.dart'; +import 'package:immich_mobile/data/store/util/cache.dart'; +import 'package:immich_mobile/domain/models/person.model.dart'; +import 'package:immich_mobile/providers/infrastructure/db.provider.dart'; +// TODO(rewrite): Remove once user metadata is a store entry of its own +import 'package:immich_mobile/providers/infrastructure/user_metadata.provider.dart'; + +/// People representing collections of faces and assets +/// +/// State is stored in the local DB, while mutations are pushed to both HTTP and DB +extension type const PersonStore._(Provider _provider) implements Provider { + /// Internal: access through `Store.people` + static final PersonStore instance = PersonStore._(Provider((ref) => PersonMutations._(ref))); + + /// Get the person specified by [personId] + /// + /// **NOTE:** This is not reactive to changes, and only hits the local DB + AutoDisposeFutureProvider byId(String personId) => _byIdProvider(personId); + + /// Get the people present in the asset [assetId] + /// + /// **NOTE:** This is not reactive to changes, and only hits the local DB + AutoDisposeFutureProvider> forAsset(String assetId) => _forAssetProvider(assetId); + + /// Get all known people, honoring the user's minimum detected face count preference + /// + /// **NOTE:** This only hits the local DB + AutoDisposeStreamProvider> all() => _allProvider; +} + +final _peopleDb = driftProvider.select((db) => db.peopleDatabaseRepository); + +// We have to map from non-reactive existing Drift queries to Riverpod reactivity, so we wrap each call in a provider family +// Note that the only reactivity here is in going from no data (fetch start) to data (fetch completed), and the DB swapping (basically never) +final _byIdProvider = FutureProvider.autoDispose.family( + (ref, personId) => ref.watch(_peopleDb).get(personId), +); + +final _forAssetProvider = FutureProvider.autoDispose.family, String>( + (ref, assetId) => ref.watch(_peopleDb).getAssetPeople(assetId), +); + +final _allProvider = StreamProvider.autoDispose>((ref) async* { + final prefs = await ref.watch(userMetadataPreferencesProvider.future); + yield* ref.watch(_peopleDb).watch(minFaces: prefs?.minimumFaces ?? 3); +}); + +class PersonMutations extends StoreMutations { + const PersonMutations._(super.ref); + + /// Update a person's name + Future updateName(String personId, String name) async { + await read(personApiRepositoryProvider).update(personId, name: name); + return read(_peopleDb).updateName(personId, name); + } + + /// Update a person's birthday + Future updateBirthday(String personId, DateTime birthday) async { + await read(personApiRepositoryProvider).update(personId, birthday: birthday); + return read(_peopleDb).updateBirthday(personId, birthday); + } +} diff --git a/mobile/lib/data/store/util/cache.dart b/mobile/lib/data/store/util/cache.dart new file mode 100644 index 000000000..44ba7c50f --- /dev/null +++ b/mobile/lib/data/store/util/cache.dart @@ -0,0 +1,164 @@ +import 'package:flutter/foundation.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; + +/// A reactive query of list data from a particular cache. This is scoped to the provided arguments +typedef CacheListQuery = + AutoDisposeFamilyAsyncNotifierProvider, List, TScope>; + +/// A future backed in-memory cache of lists of [TData], scoped by [TScope] +/// +/// For every unique scope, the future is fetched on first listen and disposed when there are no more listeners. +/// Mutations are applied using [CachedStoreMutations.cacheUpsert] and [CachedStoreMutations.cacheRemove] +/// +/// **NOTE:** Not intended for use with Drift tables; they provide their own persistence +abstract class StoreCache { + /// Fetch the contents of [scope] + @protected + Future> fetch(Ref ref, TScope scope); + + /// The identity of [item] + @protected + Object identity(TData item); + + /// Whether the corresponding list for [scope] should contain [item] + /// + /// This is checked across all currently active scopes to keep each cache up to date with the latest changes. Each + /// cache may change independently, thus we must verify that a given item is relevant to it + @protected + bool shouldContain(TScope scope, TData item); + + /// The provider family backing this cache + late final family = AsyncNotifierProvider.autoDispose.family, List, TScope>( + () => CacheNotifier._(this), + ); + + /// All active scopes, by count of live provider elements (one per container watching the scope) + final Map _liveScopes = {}; + + /// Register a live provider element for [scope] + void _addScope(TScope scope) { + _liveScopes.update(scope, (count) => count + 1, ifAbsent: () => 1); + } + + /// Unregister a live provider element for [scope] + void _removeScope(TScope scope) { + final count = _liveScopes[scope] ?? 0; + if (count <= 1) { + _liveScopes.remove(scope); + } else { + _liveScopes[scope] = count - 1; + } + } + + /// Patch and upsert in all scopes that will allow [item] given the result from [shouldContain] + void _upsert(Ref ref, TData item) { + final id = identity(item); + _patchLive( + ref, + (scope, list) => shouldContain(scope, item) ? list._upsertWhere((i) => identity(i) == id, item) : list, + ); + } + + /// Patch and remove [id] from all scopes that contain it + void _remove(Ref ref, Object id) { + _patchLive(ref, (scope, list) => list._without((i) => identity(i) == id)); + } + + /// Apply [transform] to all scopes + void _patchLive(Ref ref, List Function(TScope scope, List current) transform) { + for (final scope in [..._liveScopes.keys]) { + // Guard against scopes registered by a different ProviderContainer (tests) + if (ref.exists(family(scope))) { + ref.read(family(scope).notifier)._patch((current) => transform(scope, current)); + } + } + } +} + +/// A Riverpod [Notifier] built for handling an in-memory cache +class CacheNotifier extends AutoDisposeFamilyAsyncNotifier, Arg> { + final StoreCache _cache; + + CacheNotifier._(this._cache); + + @override + Future> build(Arg arg) { + // Track each consumer of this notifier + // Theoretically this could be called from different scopes (tests), so actually reference counting instead of relying on + // Riverpod's deduping is necessary + _cache._addScope(arg); + ref.onDispose(() => _cache._removeScope(arg)); + + return _cache.fetch(ref, arg); + } + + /// Applies [transform] to the currently stored list + /// + /// If [transform] returns the same list, no state mutation occurs + void _patch(List Function(List current) transform) { + final current = state.valueOrNull; + if (current == null) { + return; + } + + final result = transform(current); + if (!identical(result, current)) { + state = AsyncData(result); + } + } +} + +/// Base for a store entry's mutations, to be exposed by the entry's provider via `ref.read(Store.x).doMutation()` +abstract class StoreMutations { + final Ref _ref; + + const StoreMutations(this._ref); + + /// Read the state associated with a [provider] + @protected + T read(ProviderListenable provider) => _ref.read(provider); +} + +/// Mutations for a store entry whose reads are backed by a [StoreCache] +/// +/// Apply each mutation to the cache with [cacheUpsert] or [cacheRemove] +abstract class CachedStoreMutations extends StoreMutations { + final StoreCache _cache; + + const CachedStoreMutations(super.ref, this._cache); + + /// Insert or update [item] in every live scope that [StoreCache.shouldContain] it + @protected + void cacheUpsert(T item) => _cache._upsert(_ref, item); + + /// Remove the item with identity [id] from every live scope + @protected + void cacheRemove(Object id) => _cache._remove(_ref, id); +} + +/// Identity-preserving list edits +extension _ListEdits on List { + /// Upsert [item] in the location specified by [matcher], or to the end if it is not found. Preserves list identity if there are no changes + List _upsertWhere(bool Function(T item) matcher, T item) { + final index = indexWhere(matcher); + + if (index == -1) { + // Append + return [...this, item]; + } else if (this[index] != item) { + // Update + final list = [...this]; + list[index] = item; + return list; + } else { + // Do nothing + return this; + } + } + + /// Filter out items matching [matcher]. Preserves list identity if there are no changes + List _without(bool Function(T item) matcher) { + final result = where((item) => !matcher(item)).toList(); + return result.length == length ? this : result; + } +} diff --git a/mobile/lib/domain/services/people.service.dart b/mobile/lib/domain/services/people.service.dart deleted file mode 100644 index 338523f76..000000000 --- a/mobile/lib/domain/services/people.service.dart +++ /dev/null @@ -1,35 +0,0 @@ -import 'dart:async'; - -import 'package:immich_mobile/data/db/main/dao/person.dart'; -import 'package:immich_mobile/data/server/person.dart'; -import 'package:immich_mobile/domain/models/person.model.dart'; - -/// Accesses People; entities mapped to assets for presence and face detection -class PeopleService { - final PeopleRepository _repository; - final PersonApiRepository _personApiRepository; - - const PeopleService(this._repository, this._personApiRepository); - - Future get(String personId) { - return _repository.get(personId); - } - - Future> getAssetPeople(String assetId) { - return _repository.getAssetPeople(assetId); - } - - Stream> watch({int minFaces = 3}) { - return _repository.watch(minFaces: minFaces); - } - - Future updateName(String personId, String name) async { - await _personApiRepository.update(personId, name: name); - return _repository.updateName(personId, name); - } - - Future updateBirthday(String personId, DateTime birthday) async { - await _personApiRepository.update(personId, birthday: birthday); - return _repository.updateBirthday(personId, birthday); - } -} diff --git a/mobile/lib/models/activities/activity.model.dart b/mobile/lib/models/activities/activity.model.dart index 9a589ef72..742327535 100644 --- a/mobile/lib/models/activities/activity.model.dart +++ b/mobile/lib/models/activities/activity.model.dart @@ -10,6 +10,7 @@ enum ActivityType { comment, like } abstract class Activity with _$Activity { const factory Activity({ required String id, + required String albumId, String? assetId, String? comment, required DateTime createdAt, diff --git a/mobile/lib/presentation/pages/activities.page.dart b/mobile/lib/presentation/pages/activities.page.dart index 70d65675c..c06fa85cd 100644 --- a/mobile/lib/presentation/pages/activities.page.dart +++ b/mobile/lib/presentation/pages/activities.page.dart @@ -4,12 +4,12 @@ import 'package:auto_route/auto_route.dart'; import 'package:flutter/material.dart'; import 'package:flutter_hooks/flutter_hooks.dart' hide Store; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/store.dart'; import 'package:immich_mobile/domain/models/album/album.model.dart'; import 'package:immich_mobile/extensions/asyncvalue_extensions.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/like_activity_action_button.widget.dart'; import 'package:immich_mobile/presentation/widgets/album/activity_text_field.dart'; -import 'package:immich_mobile/providers/activity.provider.dart'; import 'package:immich_mobile/providers/infrastructure/current_album.provider.dart'; import 'package:immich_mobile/widgets/activities/comment_bubble.dart'; @@ -23,8 +23,7 @@ class ActivitiesPage extends HookConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { - final activityNotifier = ref.watch(albumActivityProvider((album.id, assetId)).notifier); - final activities = ref.watch(albumActivityProvider((album.id, assetId))); + final activities = ref.watch(Store.activity.list(album.id, assetId: assetId)); final listViewScrollController = useScrollController(); Future scrollToBottom() { @@ -36,7 +35,12 @@ class ActivitiesPage extends HookConsumerWidget { } Future onAddComment(String comment) async { - await activityNotifier.addComment(comment); + try { + await ref.read(Store.activity).addComment(album.id, comment, assetId: assetId); + } catch (e) { + // TODO(rewrite): Actually handle this + } + unawaited(scrollToBottom()); } diff --git a/mobile/lib/presentation/pages/library.page.dart b/mobile/lib/presentation/pages/library.page.dart index 1c032cba5..12a38976b 100644 --- a/mobile/lib/presentation/pages/library.page.dart +++ b/mobile/lib/presentation/pages/library.page.dart @@ -1,6 +1,7 @@ import 'package:auto_route/auto_route.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/store.dart'; import 'package:immich_mobile/domain/models/user.model.dart'; import 'package:immich_mobile/extensions/asyncvalue_extensions.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; @@ -11,7 +12,6 @@ import 'package:immich_mobile/presentation/widgets/images/thumbnail.widget.dart' import 'package:immich_mobile/presentation/widgets/people/partner_user_avatar.widget.dart'; import 'package:immich_mobile/providers/infrastructure/album.provider.dart'; import 'package:immich_mobile/providers/infrastructure/memory.provider.dart'; -import 'package:immich_mobile/providers/infrastructure/people.provider.dart'; import 'package:immich_mobile/providers/infrastructure/user.provider.dart'; import 'package:immich_mobile/providers/server_info.provider.dart'; import 'package:immich_mobile/providers/user.provider.dart'; @@ -152,7 +152,7 @@ class _PeopleCollectionCard extends ConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { - final people = ref.watch(getAllPeopleProvider); + final people = ref.watch(Store.people.all()); return LayoutBuilder( builder: (context, constraints) { diff --git a/mobile/lib/presentation/pages/people_collection.page.dart b/mobile/lib/presentation/pages/people_collection.page.dart index c03f5157b..e7e59c73f 100644 --- a/mobile/lib/presentation/pages/people_collection.page.dart +++ b/mobile/lib/presentation/pages/people_collection.page.dart @@ -3,12 +3,12 @@ import 'dart:async'; import 'package:auto_route/auto_route.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/store.dart'; import 'package:immich_mobile/domain/models/person.model.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; import 'package:immich_mobile/extensions/string_extensions.dart'; import 'package:immich_mobile/generated/translations.g.dart'; import 'package:immich_mobile/presentation/widgets/images/remote_image_provider.dart'; -import 'package:immich_mobile/providers/infrastructure/people.provider.dart'; import 'package:immich_mobile/routing/router.dart'; import 'package:immich_mobile/utils/image_url_builder.dart'; import 'package:immich_mobile/utils/people.utils.dart'; @@ -34,7 +34,7 @@ class _PeopleCollectionPageState extends ConsumerState { @override Widget build(BuildContext context) { - final people = ref.watch(getAllPeopleProvider); + final people = ref.watch(Store.people.all()); return LayoutBuilder( builder: (context, constraints) { diff --git a/mobile/lib/presentation/widgets/action_buttons/like_activity_action_button.widget.dart b/mobile/lib/presentation/widgets/action_buttons/like_activity_action_button.widget.dart index e39a76ef0..6537fbbce 100644 --- a/mobile/lib/presentation/widgets/action_buttons/like_activity_action_button.widget.dart +++ b/mobile/lib/presentation/widgets/action_buttons/like_activity_action_button.widget.dart @@ -1,11 +1,11 @@ import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/store.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/generated/translations.g.dart'; import 'package:immich_mobile/models/activities/activity.model.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/base_action_button.widget.dart'; -import 'package:immich_mobile/providers/activity.provider.dart'; import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.dart'; import 'package:immich_mobile/providers/infrastructure/current_album.provider.dart'; import 'package:immich_mobile/providers/user.provider.dart'; @@ -22,20 +22,22 @@ class LikeActivityActionButton extends ConsumerWidget { final asset = ref.watch(assetViewerProvider.select((s) => s.currentAsset)) as RemoteAsset?; final user = ref.watch(currentUserProvider); - final activities = ref.watch(albumActivityProvider((album?.id ?? "", asset?.id))); + final activities = ref.watch(Store.activity.list(album?.id ?? "", assetId: asset?.id)); Future onTap(Activity? liked) async { if (user == null) { return; } - if (liked != null) { - await ref.read(albumActivityProvider((album?.id ?? "", asset?.id)).notifier).removeActivity(liked.id); - } else { - await ref.read(albumActivityProvider((album?.id ?? "", asset?.id)).notifier).addLike(); + try { + if (liked != null) { + await ref.read(Store.activity).remove(liked); + } else { + await ref.read(Store.activity).addLike(album?.id ?? "", assetId: asset?.id); + } + } catch (e) { + // TODO(rewrite): Actually handle this } - - ref.invalidate(albumActivityProvider((album?.id ?? "", asset?.id))); } return activities.when( diff --git a/mobile/lib/presentation/widgets/asset_viewer/asset_details/people_details.widget.dart b/mobile/lib/presentation/widgets/asset_viewer/asset_details/people_details.widget.dart index 34ec67cc7..324910f70 100644 --- a/mobile/lib/presentation/widgets/asset_viewer/asset_details/people_details.widget.dart +++ b/mobile/lib/presentation/widgets/asset_viewer/asset_details/people_details.widget.dart @@ -3,6 +3,7 @@ import 'dart:async'; import 'package:auto_route/auto_route.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/store.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/domain/models/person.model.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; @@ -10,7 +11,6 @@ import 'package:immich_mobile/extensions/theme_extensions.dart'; import 'package:immich_mobile/generated/translations.g.dart'; import 'package:immich_mobile/presentation/widgets/images/remote_image_provider.dart'; import 'package:immich_mobile/presentation/widgets/people/person_edit_name_modal.widget.dart'; -import 'package:immich_mobile/providers/infrastructure/people.provider.dart'; import 'package:immich_mobile/providers/routes.provider.dart'; import 'package:immich_mobile/routing/router.dart'; import 'package:immich_mobile/utils/image_url_builder.dart'; @@ -28,7 +28,7 @@ class PeopleDetails extends ConsumerWidget { return const SizedBox.shrink(); } - final peopleFuture = ref.watch(peopleAssetProvider(asset.id)); + final peopleFuture = ref.watch(Store.people.forAsset(asset.id)); Future showNameEditModal(Person person) async { await showDialog( @@ -39,7 +39,8 @@ class PeopleDetails extends ConsumerWidget { }, ); - ref.invalidate(peopleAssetProvider(asset.id)); + // TODO(agg23): Remove once state is properly reactive + ref.invalidate(Store.people.forAsset(asset.id)); } return peopleFuture.when( diff --git a/mobile/lib/presentation/widgets/asset_viewer/viewer_top_app_bar.widget.dart b/mobile/lib/presentation/widgets/asset_viewer/viewer_top_app_bar.widget.dart index 994f3785e..917d5a5c9 100644 --- a/mobile/lib/presentation/widgets/asset_viewer/viewer_top_app_bar.widget.dart +++ b/mobile/lib/presentation/widgets/asset_viewer/viewer_top_app_bar.widget.dart @@ -3,6 +3,7 @@ import 'dart:async'; import 'package:auto_route/auto_route.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/store.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; import 'package:immich_mobile/extensions/datetime_extensions.dart'; @@ -10,7 +11,6 @@ import 'package:immich_mobile/presentation/actions/action.widget.dart'; import 'package:immich_mobile/presentation/actions/favorite.action.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/motion_photo_action_button.widget.dart'; import 'package:immich_mobile/presentation/widgets/asset_viewer/viewer_kebab_menu.widget.dart'; -import 'package:immich_mobile/providers/activity.provider.dart'; import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.dart'; import 'package:immich_mobile/providers/infrastructure/asset_viewer/asset.provider.dart'; import 'package:immich_mobile/providers/infrastructure/current_album.provider.dart'; @@ -38,7 +38,7 @@ class ViewerTopAppBar extends ConsumerWidget implements PreferredSizeWidget { final showingDetails = ref.watch(assetViewerProvider.select((state) => state.showingDetails)); if (album != null && album.isActivityEnabled && album.isShared && asset is RemoteAsset) { - ref.watch(albumActivityProvider((album.id, asset.id))); + ref.watch(Store.activity.list(album.id, assetId: asset.id)); } final showingControls = ref.watch(assetViewerProvider.select((s) => s.showingControls)); diff --git a/mobile/lib/presentation/widgets/people/person_edit_birthday_modal.widget.dart b/mobile/lib/presentation/widgets/people/person_edit_birthday_modal.widget.dart index 3fce4032d..55dc277cc 100644 --- a/mobile/lib/presentation/widgets/people/person_edit_birthday_modal.widget.dart +++ b/mobile/lib/presentation/widgets/people/person_edit_birthday_modal.widget.dart @@ -2,10 +2,10 @@ import 'package:easy_localization/easy_localization.dart'; import 'package:flutter/material.dart'; import 'package:fluttertoast/fluttertoast.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/store.dart'; import 'package:immich_mobile/domain/models/person.model.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; import 'package:immich_mobile/generated/translations.g.dart'; -import 'package:immich_mobile/providers/infrastructure/people.provider.dart'; import 'package:immich_mobile/utils/debug_print.dart'; import 'package:immich_mobile/widgets/common/immich_toast.dart'; import 'package:scroll_date_picker/scroll_date_picker.dart'; @@ -30,7 +30,7 @@ class _PersonBirthdayEditFormState extends ConsumerState Future saveBirthday() async { try { - final result = await ref.read(peopleServiceProvider).updateBirthday(widget.person.id, _selectedDate); + final result = await ref.read(Store.people).updateBirthday(widget.person.id, _selectedDate); if (result != 0 && mounted) { context.pop(_selectedDate); diff --git a/mobile/lib/presentation/widgets/people/person_edit_name_modal.widget.dart b/mobile/lib/presentation/widgets/people/person_edit_name_modal.widget.dart index d120f2324..f1743fb41 100644 --- a/mobile/lib/presentation/widgets/people/person_edit_name_modal.widget.dart +++ b/mobile/lib/presentation/widgets/people/person_edit_name_modal.widget.dart @@ -1,10 +1,10 @@ import 'package:flutter/material.dart'; import 'package:fluttertoast/fluttertoast.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/store.dart'; import 'package:immich_mobile/domain/models/person.model.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; import 'package:immich_mobile/generated/translations.g.dart'; -import 'package:immich_mobile/providers/infrastructure/people.provider.dart'; import 'package:immich_mobile/utils/debug_print.dart'; import 'package:immich_mobile/widgets/common/immich_toast.dart'; @@ -28,7 +28,7 @@ class _PersonNameEditFormState extends ConsumerState { Future onEdit(String personId, String newName) async { try { - final result = await ref.read(peopleServiceProvider).updateName(personId, newName); + final result = await ref.read(Store.people).updateName(personId, newName); if (result != 0 && mounted) { context.pop(newName); } diff --git a/mobile/lib/providers/activity.provider.dart b/mobile/lib/providers/activity.provider.dart deleted file mode 100644 index b2cdbcf18..000000000 --- a/mobile/lib/providers/activity.provider.dart +++ /dev/null @@ -1,81 +0,0 @@ -import 'package:collection/collection.dart'; -import 'package:hooks_riverpod/hooks_riverpod.dart'; -import 'package:immich_mobile/models/activities/activity.model.dart'; -import 'package:immich_mobile/providers/activity_service.provider.dart'; - -// ignore: unintended_html_in_doc_comment -/// Maintains the current list of all activities for - -final albumActivityProvider = AsyncNotifierProvider.autoDispose - .family, (String albumId, String? assetId)>(AlbumActivity.new); - -class AlbumActivity extends AutoDisposeFamilyAsyncNotifier, (String albumId, String? assetId)> { - late String albumId; - late String? assetId; - - @override - Future> build((String albumId, String? assetId) args) async { - albumId = args.$1; - assetId = args.$2; - return ref.watch(activityServiceProvider).getAllActivities(albumId, assetId: assetId); - } - - Future removeActivity(String id) async { - if (await ref.watch(activityServiceProvider).removeActivity(id)) { - final removedActivity = _removeFromState(id); - if (removedActivity == null) { - return; - } - - if (assetId != null) { - ref.read(albumActivityProvider((albumId, assetId)).notifier)._removeFromState(id); - } - } - } - - Future addLike() async { - final activity = await ref.watch(activityServiceProvider).addActivity(albumId, ActivityType.like, assetId: assetId); - if (activity.hasValue) { - _addToState(activity.requireValue); - if (assetId != null) { - ref.read(albumActivityProvider((albumId, assetId)).notifier)._addToState(activity.requireValue); - } - } - } - - Future addComment(String comment) async { - final activity = await ref - .watch(activityServiceProvider) - .addActivity(albumId, ActivityType.comment, assetId: assetId, comment: comment); - - if (activity.hasValue) { - _addToState(activity.requireValue); - if (assetId != null) { - ref.read(albumActivityProvider((albumId, assetId)).notifier)._addToState(activity.requireValue); - } - } - } - - void _addToState(Activity activity) { - final activities = state.valueOrNull ?? []; - if (activities.any((a) => a.id == activity.id)) { - return; - } - state = AsyncData([...activities, activity]); - } - - Activity? _removeFromState(String id) { - final activities = state.valueOrNull; - if (activities == null) { - return null; - } - final activity = activities.firstWhereOrNull((a) => a.id == id); - if (activity == null) { - return null; - } - - final updated = [...activities]..remove(activity); - state = AsyncData(updated); - return activity; - } -} diff --git a/mobile/lib/providers/activity_service.provider.dart b/mobile/lib/providers/activity_service.provider.dart deleted file mode 100644 index 3be6c6b23..000000000 --- a/mobile/lib/providers/activity_service.provider.dart +++ /dev/null @@ -1,13 +0,0 @@ -import 'package:hooks_riverpod/hooks_riverpod.dart'; -import 'package:immich_mobile/providers/infrastructure/asset.provider.dart'; -import 'package:immich_mobile/providers/infrastructure/timeline.provider.dart'; -import 'package:immich_mobile/repositories/activity_api.repository.dart'; -import 'package:immich_mobile/services/activity.service.dart'; - -final activityServiceProvider = Provider.autoDispose((ref) { - return ActivityService( - ref.watch(activityApiRepositoryProvider), - ref.watch(timelineFactoryProvider), - ref.watch(assetServiceProvider), - ); -}); diff --git a/mobile/lib/providers/infrastructure/people.provider.dart b/mobile/lib/providers/infrastructure/people.provider.dart deleted file mode 100644 index bc27f646f..000000000 --- a/mobile/lib/providers/infrastructure/people.provider.dart +++ /dev/null @@ -1,21 +0,0 @@ -import 'package:hooks_riverpod/hooks_riverpod.dart'; -import 'package:immich_mobile/data/server/person.dart'; -import 'package:immich_mobile/domain/models/person.model.dart'; -import 'package:immich_mobile/domain/services/people.service.dart'; -import 'package:immich_mobile/providers/infrastructure/db.provider.dart'; -import 'package:immich_mobile/providers/infrastructure/user_metadata.provider.dart'; - -final peopleServiceProvider = Provider( - (ref) => PeopleService(ref.watch(driftProvider).peopleRepository, ref.watch(personApiRepositoryProvider)), -); - -final peopleAssetProvider = FutureProvider.family, String>((ref, assetId) async { - final service = ref.watch(peopleServiceProvider); - return service.getAssetPeople(assetId); -}); - -final getAllPeopleProvider = StreamProvider>((ref) async* { - final service = ref.watch(peopleServiceProvider); - final prefs = await ref.watch(userMetadataPreferencesProvider.future); - yield* service.watch(minFaces: prefs?.minimumFaces ?? 3); -}); diff --git a/mobile/lib/services/activity.service.dart b/mobile/lib/services/activity.service.dart deleted file mode 100644 index 5c60a6a04..000000000 --- a/mobile/lib/services/activity.service.dart +++ /dev/null @@ -1,66 +0,0 @@ -import 'package:hooks_riverpod/hooks_riverpod.dart'; -import 'package:immich_mobile/data/server/errors.dart'; -import 'package:immich_mobile/domain/services/asset.service.dart'; -import 'package:immich_mobile/domain/services/timeline.service.dart'; -import 'package:immich_mobile/mixins/error_logger.mixin.dart'; -import 'package:immich_mobile/models/activities/activity.model.dart'; -import 'package:immich_mobile/presentation/widgets/asset_viewer/asset_viewer.page.dart'; -import 'package:immich_mobile/providers/infrastructure/current_album.provider.dart'; -import 'package:immich_mobile/repositories/activity_api.repository.dart'; -import 'package:immich_mobile/routing/router.dart'; -import 'package:logging/logging.dart'; - -class ActivityService with ErrorLoggerMixin { - final ActivityApiRepository _activityApiRepository; - final TimelineFactory _timelineFactory; - final AssetService _assetService; - - @override - final Logger logger = Logger("ActivityService"); - - ActivityService(this._activityApiRepository, this._timelineFactory, this._assetService); - - Future> getAllActivities(String albumId, {String? assetId}) async { - return logError( - () => _activityApiRepository.getAll(albumId, assetId: assetId), - defaultValue: [], - errorMessage: "Failed to get all activities for album $albumId", - ); - } - - Future removeActivity(String id) async { - return logError( - () async { - try { - await _activityApiRepository.delete(id); - } on NoResponseDtoError { - return true; - } - return true; - }, - defaultValue: false, - errorMessage: "Failed to delete activity", - ); - } - - AsyncFuture addActivity(String albumId, ActivityType type, {String? assetId, String? comment}) async { - return guardError( - () => _activityApiRepository.create(albumId, type, assetId: assetId, comment: comment), - errorMessage: "Failed to create $type for album $albumId", - ); - } - - Future buildAssetViewerRoute(String assetId, WidgetRef ref) async { - final asset = await _assetService.getRemoteAsset(assetId); - if (asset == null) { - return null; - } - - AssetViewer.setAsset(ref, asset); - return AssetViewerRoute( - initialIndex: 0, - timelineService: _timelineFactory.fromAssets([asset], TimelineOrigin.albumActivities), - currentAlbum: ref.read(currentRemoteAlbumProvider), - ); - } -} diff --git a/mobile/lib/services/deep_link.service.dart b/mobile/lib/services/deep_link.service.dart index 2f593ffba..76aaf8bc6 100644 --- a/mobile/lib/services/deep_link.service.dart +++ b/mobile/lib/services/deep_link.service.dart @@ -1,17 +1,16 @@ import 'package:auto_route/auto_route.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/store.dart'; import 'package:immich_mobile/domain/models/memory.model.dart'; import 'package:immich_mobile/domain/models/user.model.dart'; import 'package:immich_mobile/domain/services/asset.service.dart' as beta_asset_service; import 'package:immich_mobile/domain/services/memory.service.dart'; -import 'package:immich_mobile/domain/services/people.service.dart'; import 'package:immich_mobile/domain/services/remote_album.service.dart'; import 'package:immich_mobile/domain/services/timeline.service.dart'; import 'package:immich_mobile/presentation/widgets/asset_viewer/asset_viewer.page.dart'; import 'package:immich_mobile/providers/infrastructure/album.provider.dart'; import 'package:immich_mobile/providers/infrastructure/asset.provider.dart' as beta_asset_provider; import 'package:immich_mobile/providers/infrastructure/db.provider.dart'; -import 'package:immich_mobile/providers/infrastructure/people.provider.dart'; import 'package:immich_mobile/providers/infrastructure/timeline.provider.dart'; import 'package:immich_mobile/providers/user.provider.dart'; import 'package:immich_mobile/routing/router.dart'; @@ -22,7 +21,6 @@ final deepLinkServiceProvider = Provider( ref.watch(beta_asset_provider.assetServiceProvider), ref.watch(remoteAlbumServiceProvider), MemoryService(ref.watch(driftProvider).memoryRepository), - ref.watch(peopleServiceProvider), ref.watch(currentUserProvider), ), ); @@ -32,7 +30,6 @@ class DeepLinkService { final beta_asset_service.AssetService _betaAssetService; final RemoteAlbumService _betaRemoteAlbumService; final MemoryService _betaMemoryService; - final PeopleService _betaPeopleService; final UserDto? _currentUser; @@ -41,7 +38,6 @@ class DeepLinkService { this._betaAssetService, this._betaRemoteAlbumService, this._betaMemoryService, - this._betaPeopleService, this._currentUser, ); @@ -54,7 +50,7 @@ class DeepLinkService { "memory" => await _buildMemoryDeepLink(queryParams['id']), "asset" => await _buildAssetDeepLink(queryParams['id'] ?? '', ref), "album" => await _buildAlbumDeepLink(queryParams['id'] ?? ''), - "people" => await _buildPeopleDeepLink(queryParams['id'] ?? ''), + "people" => await _buildPeopleDeepLink(queryParams['id'] ?? '', ref), "activity" => await _buildActivityDeepLink(queryParams['albumId'] ?? ''), _ => null, }; @@ -81,7 +77,7 @@ class DeepLinkService { } if (peopleRegex.hasMatch(path)) { final peopleId = peopleRegex.firstMatch(path)?.group(1) ?? ''; - return _buildPeopleDeepLink(peopleId); + return _buildPeopleDeepLink(peopleId, ref); } return null; @@ -146,8 +142,8 @@ class DeepLinkService { return ActivitiesRoute(album: album); } - Future _buildPeopleDeepLink(String personId) async { - final person = await _betaPeopleService.get(personId); + Future _buildPeopleDeepLink(String personId, WidgetRef ref) async { + final person = await ref.read(Store.people.byId(personId).future); if (person == null) { return null; diff --git a/mobile/lib/utils/provider_utils.dart b/mobile/lib/utils/provider_utils.dart index 861167e20..56a9a4bfd 100644 --- a/mobile/lib/utils/provider_utils.dart +++ b/mobile/lib/utils/provider_utils.dart @@ -1,8 +1,8 @@ import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/server/activity.dart'; import 'package:immich_mobile/data/server/person.dart'; import 'package:immich_mobile/providers/infrastructure/search.provider.dart'; import 'package:immich_mobile/providers/infrastructure/user.provider.dart'; -import 'package:immich_mobile/repositories/activity_api.repository.dart'; import 'package:immich_mobile/repositories/album_api_repository.dart'; import 'package:immich_mobile/repositories/asset_api.repository.dart'; import 'package:immich_mobile/repositories/partner_api.repository.dart'; diff --git a/mobile/lib/widgets/activities/comment_bubble.dart b/mobile/lib/widgets/activities/comment_bubble.dart index 543142074..3d9f8fb19 100644 --- a/mobile/lib/widgets/activities/comment_bubble.dart +++ b/mobile/lib/widgets/activities/comment_bubble.dart @@ -1,14 +1,18 @@ import 'package:auto_route/auto_route.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/store.dart'; +import 'package:immich_mobile/domain/services/timeline.service.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; import 'package:immich_mobile/extensions/datetime_extensions.dart'; import 'package:immich_mobile/models/activities/activity.model.dart'; +import 'package:immich_mobile/presentation/widgets/asset_viewer/asset_viewer.page.dart'; import 'package:immich_mobile/presentation/widgets/images/remote_image_provider.dart'; -import 'package:immich_mobile/providers/activity.provider.dart'; -import 'package:immich_mobile/providers/activity_service.provider.dart'; +import 'package:immich_mobile/providers/infrastructure/asset.provider.dart'; import 'package:immich_mobile/providers/infrastructure/current_album.provider.dart'; +import 'package:immich_mobile/providers/infrastructure/timeline.provider.dart'; import 'package:immich_mobile/providers/user.provider.dart'; +import 'package:immich_mobile/routing/router.dart'; import 'package:immich_mobile/widgets/activities/dismissible_activity.dart'; import 'package:immich_mobile/widgets/common/user_circle_avatar.dart'; @@ -28,19 +32,27 @@ class CommentBubble extends ConsumerWidget { final isLike = activity.type == ActivityType.like; final bgColor = isOwn ? context.colorScheme.primaryContainer : context.colorScheme.surfaceContainer; - final activityNotifier = ref.watch( - albumActivityProvider((album.id, isAssetActivity ? activity.assetId : null)).notifier, - ); - Future openAssetViewer() async { - final activityService = ref.read(activityServiceProvider); - final route = await activityService.buildAssetViewerRoute(activity.assetId!, ref); - if (!context.mounted) { + final asset = await ref.read(assetServiceProvider).getRemoteAsset(activity.assetId!); + if (asset == null || !context.mounted) { return; } - if (route != null) { - await context.pushRoute(route); + AssetViewer.setAsset(ref, asset); + await context.pushRoute( + AssetViewerRoute( + initialIndex: 0, + timelineService: ref.read(timelineFactoryProvider).fromAssets([asset], TimelineOrigin.albumActivities), + currentAlbum: ref.read(currentRemoteAlbumProvider), + ), + ); + } + + Future delete() async { + try { + await ref.read(Store.activity).remove(activity); + } catch (e) { + // TODO(rewrite): Actually handle this } } @@ -112,7 +124,7 @@ class CommentBubble extends ConsumerWidget { final List contentChildren = [thumbnail, likes, commentBubble].whereType().toList(); return DismissibleActivity( - onDismiss: canDelete ? (id) async => await activityNotifier.removeActivity(id) : null, + onDismiss: canDelete ? (id) => delete() : null, activity.id, Align( alignment: isOwn ? Alignment.centerRight : Alignment.centerLeft, diff --git a/mobile/lib/widgets/search/search_filter/people_picker.dart b/mobile/lib/widgets/search/search_filter/people_picker.dart index 2e380bb23..72bd8c994 100644 --- a/mobile/lib/widgets/search/search_filter/people_picker.dart +++ b/mobile/lib/widgets/search/search_filter/people_picker.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; -import 'package:flutter_hooks/flutter_hooks.dart'; +import 'package:flutter_hooks/flutter_hooks.dart' hide Store; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/store.dart'; import 'package:immich_mobile/domain/models/person.model.dart'; import 'package:immich_mobile/extensions/asyncvalue_extensions.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; @@ -8,7 +9,6 @@ import 'package:immich_mobile/extensions/string_extensions.dart'; import 'package:immich_mobile/generated/translations.g.dart'; import 'package:immich_mobile/pages/common/large_leading_tile.dart'; import 'package:immich_mobile/presentation/widgets/images/remote_image_provider.dart'; -import 'package:immich_mobile/providers/infrastructure/people.provider.dart'; import 'package:immich_mobile/utils/image_url_builder.dart'; import 'package:immich_mobile/widgets/common/search_field.dart'; @@ -23,7 +23,7 @@ class PeoplePicker extends HookConsumerWidget { final formFocus = useFocusNode(); const imageSize = 60.0; final searchQuery = useState(''); - final people = ref.watch(getAllPeopleProvider); + final people = ref.watch(Store.people.all()); final selectedPeople = useState>({...?initialSelection}); return Column( diff --git a/mobile/test/data/store/activity_store_test.dart b/mobile/test/data/store/activity_store_test.dart new file mode 100644 index 000000000..e3273f628 --- /dev/null +++ b/mobile/test/data/store/activity_store_test.dart @@ -0,0 +1,86 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/server/activity.dart'; +import 'package:immich_mobile/data/server/errors.dart'; +import 'package:immich_mobile/data/store.dart'; +import 'package:immich_mobile/domain/models/user.model.dart'; +import 'package:immich_mobile/models/activities/activity.model.dart'; +import 'package:mocktail/mocktail.dart'; + +class _MockActivityApi extends Mock implements ActivityApiRepository {} + +final _user = UserDto(id: 'u1', email: 'u1@test.com', name: 'User', profileChangedAt: DateTime.utc(2025)); + +Activity _activity(String id, {String? assetId, ActivityType type = ActivityType.comment}) => + Activity(id: id, albumId: 'album', assetId: assetId, createdAt: DateTime.utc(2026), type: type, user: _user); + +void main() { + late _MockActivityApi api; + late ProviderContainer container; + + setUp(() { + api = _MockActivityApi(); + container = ProviderContainer(overrides: [activityApiRepositoryProvider.overrideWithValue(api)]); + addTearDown(container.dispose); + + // Fetches for any ID default to empty + when(() => api.getAll(any(), assetId: any(named: 'assetId'))).thenAnswer((_) async => []); + }); + + Future load(String albumId, {String? assetId}) async { + // Create a lock on the provider scope so it doesn't get disposed + container.listen(Store.activity.list(albumId, assetId: assetId), (_, _) {}); + await container.read(Store.activity.list(albumId, assetId: assetId).future); + } + + List? current(String albumId, {String? assetId}) => + container.read(Store.activity.list(albumId, assetId: assetId)).value; + + test('list serves fetched activities', () async { + when(() => api.getAll('album', assetId: null)).thenAnswer((_) async => [_activity('1')]); + when(() => api.getAll('broken', assetId: null)).thenThrow(Exception('offline')); + + await load('album'); + await load('broken'); + + expect(current('album'), [_activity('1')]); + expect(current('broken'), isEmpty); + }); + + test('addComment appends the created activity to the live album and asset lists', () async { + await load('album'); + await load('album', assetId: 'asset'); + + final created = _activity('c1', assetId: 'asset'); + when( + () => api.create('album', ActivityType.comment, assetId: 'asset', comment: 'hi'), + ).thenAnswer((_) async => created); + + await container.read(Store.activity).addComment('album', 'hi', assetId: 'asset'); + + expect(current('album'), [created]); + expect(current('album', assetId: 'asset'), [created]); + }); + + test('addLike appends the created like to the live album list', () async { + await load('album'); + final created = _activity('l1', type: ActivityType.like); + when(() => api.create('album', ActivityType.like, assetId: null)).thenAnswer((_) async => created); + + await container.read(Store.activity).addLike('album'); + + expect(current('album'), [created]); + }); + + test('remove deletes the activity', () async { + when(() => api.getAll('album', assetId: null)).thenAnswer((_) async => [_activity('1')]); + await load('album'); + + // The delete endpoint sends no response body, producing an error on our side + when(() => api.delete('1')).thenThrow(const NoResponseDtoError()); + + await container.read(Store.activity).remove(_activity('1')); + + expect(current('album'), isEmpty); + }); +} diff --git a/mobile/test/data/store/person_store_test.dart b/mobile/test/data/store/person_store_test.dart new file mode 100644 index 000000000..20571b500 --- /dev/null +++ b/mobile/test/data/store/person_store_test.dart @@ -0,0 +1,97 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/server/person.dart'; +import 'package:immich_mobile/data/store.dart'; +import 'package:immich_mobile/domain/models/person.model.dart'; +import 'package:immich_mobile/providers/infrastructure/db.provider.dart'; +import 'package:immich_mobile/providers/infrastructure/user_metadata.provider.dart'; +import 'package:mocktail/mocktail.dart'; + +import '../../medium/repository_context.dart'; + +class _MockPersonApi extends Mock implements PersonApiRepository {} + +void main() { + late MediumRepositoryContext ctx; + late _MockPersonApi api; + late ProviderContainer container; + + setUp(() { + ctx = MediumRepositoryContext(); + api = _MockPersonApi(); + container = ProviderContainer( + overrides: [ + driftProvider.overrideWithValue(ctx.db), + personApiRepositoryProvider.overrideWithValue(api), + // No stored preferences: the default minimum face count applies + userMetadataPreferencesProvider.overrideWith((ref) => Future.value(null)), + ], + ); + addTearDown(container.dispose); + addTearDown(ctx.dispose); + }); + + Future byId(String personId) => container.read(Store.people.byId(personId).future); + + test('forAsset serves the people on the asset', () async { + final user = await ctx.newUser(); + final asset = await ctx.newRemoteAsset(ownerId: user.id); + final person = await ctx.newPerson(ownerId: user.id); + await ctx.newFace(assetId: asset.id, personId: person.id); + + final people = await container.read(Store.people.forAsset(asset.id).future); + + expect(people.map((p) => p.id), [person.id]); + }); + + test('all serves named people, hiding unnamed ones lacking the face count', () async { + final user = await ctx.newUser(); + final asset = await ctx.newRemoteAsset(ownerId: user.id); + final named = await ctx.newPerson(ownerId: user.id, name: 'Alice'); + final unnamed = await ctx.newPerson(ownerId: user.id, name: ''); + + await ctx.newFace(assetId: asset.id, personId: named.id); + await ctx.newFace(assetId: asset.id, personId: unnamed.id); + + final people = await container.read(Store.people.all().future); + + expect(people.map((p) => p.id), [named.id]); + }); + + test('updateName pushes to the server, then saves locally', () async { + final user = await ctx.newUser(); + final person = await ctx.newPerson(ownerId: user.id, name: 'Old'); + + when(() => api.update(person.id, name: 'New')).thenAnswer((_) async => Person(id: person.id, name: 'New')); + + await container.read(Store.people).updateName(person.id, 'New'); + + verify(() => api.update(person.id, name: 'New')).called(1); + expect((await byId(person.id))?.name, 'New'); + }); + + test('updateName leaves the DB untouched when the server rejects', () async { + final user = await ctx.newUser(); + final person = await ctx.newPerson(ownerId: user.id, name: 'Old'); + when(() => api.update(person.id, name: 'New')).thenThrow(Exception('rejected')); + + await expectLater(container.read(Store.people).updateName(person.id, 'New'), throwsException); + + expect((await byId(person.id))?.name, 'Old'); + }); + + test('updateBirthday pushes to the server, then saves locally', () async { + final user = await ctx.newUser(); + final person = await ctx.newPerson(ownerId: user.id); + final birthday = DateTime.utc(1990, 4, 2); + + when( + () => api.update(person.id, birthday: birthday), + ).thenAnswer((_) async => Person(id: person.id, name: person.name, birthDate: birthday)); + + await container.read(Store.people).updateBirthday(person.id, birthday); + + verify(() => api.update(person.id, birthday: birthday)).called(1); + expect((await byId(person.id))?.birthDate, birthday); + }); +} diff --git a/mobile/test/data/store/util/cache_test.dart b/mobile/test/data/store/util/cache_test.dart new file mode 100644 index 000000000..d905a92dc --- /dev/null +++ b/mobile/test/data/store/util/cache_test.dart @@ -0,0 +1,146 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/data/store/util/cache.dart'; + +typedef _Item = ({String id, String scope, int version}); + +_Item _item(String id, String scope, [int version = 1]) => (id: id, scope: scope, version: version); + +class _TestCache extends StoreCache<_Item, String> { + final fetchLog = []; + final seed = >{}; + + @override + Future> fetch(Ref ref, String scope) async { + fetchLog.add(scope); + return seed[scope] ?? const []; + } + + @override + Object identity(_Item item) => item.id; + + @override + bool shouldContain(String scope, _Item item) => item.scope == scope; +} + +class _TestMutations extends CachedStoreMutations<_Item, String> { + const _TestMutations(super.ref, super.cache); + + void upsert(_Item item) => cacheUpsert(item); + + void removeById(Object id) => cacheRemove(id); +} + +void main() { + late _TestCache cache; + late ProviderContainer container; + late _TestMutations mutations; + + setUp(() { + cache = _TestCache(); + container = ProviderContainer(); + mutations = container.read(Provider((ref) => _TestMutations(ref, cache))); + addTearDown(container.dispose); + }); + + /// Keeps [scope] alive in [target] for the test's duration, collecting state changes + List>> listen(String scope, [ProviderContainer? target]) { + final states = >>[]; + (target ?? container).listen(cache.family(scope), (_, next) => states.add(next)); + return states; + } + + Future> load(String scope, [ProviderContainer? target]) => + (target ?? container).read(cache.family(scope).future); + + List<_Item>? current(String scope) => container.read(cache.family(scope)).value; + + test('fetches once per live scope, and refetches on new subscription after its last listener disconnected', () async { + final first = container.listen(cache.family('a'), (_, _) {}); + final second = container.listen(cache.family('a'), (_, _) {}); + + await load('a'); + expect(cache.fetchLog, ['a']); + + first.close(); + await container.pump(); + expect(cache.fetchLog, ['a']); + + second.close(); + await container.pump(); + + listen('a'); + await load('a'); + + expect(cache.fetchLog, ['a', 'a']); + }); + + test('upsert inserts or updates by identity, limited to shouldContain', () async { + cache.seed['a'] = [_item('1', 'a', 1)]; + listen('a'); + listen('b'); + + await load('a'); + await load('b'); + + // Update existing + mutations.upsert(_item('1', 'a', 2)); + + // Insert new + mutations.upsert(_item('2', 'a')); + + // No scope + mutations.upsert(_item('3', 'c')); + + expect(current('a'), [_item('1', 'a', 2), _item('2', 'a')]); + expect(current('b'), isEmpty); + + expect(cache.fetchLog, ['a', 'b']); + }); + + test('remove drops the item from all live scopes', () async { + cache.seed['a'] = [_item('x', 'a')]; + cache.seed['b'] = [_item('x', 'b'), _item('y', 'b')]; + + listen('a'); + listen('b'); + await load('a'); + await load('b'); + + mutations.removeById('x'); + + expect(current('a'), isEmpty); + expect(current('b'), [_item('y', 'b')]); + }); + + test('patches that change nothing do not emit', () async { + cache.seed['a'] = [_item('1', 'a')]; + + final states = listen('a'); + await load('a'); + final oldStateCount = states.length; + + mutations.upsert(_item('1', 'a')); + mutations.removeById('nope'); + + expect(states.length, oldStateCount); + }); + + test('patches scopes that are live in other Riverpod containers', () async { + final other = ProviderContainer(); + + listen('a', other); + await load('a', other); + + listen('a'); + await load('a'); + + // We kill the external container, but ours (the second reference) should still work + other.dispose(); + await container.pump(); + + mutations.upsert(_item('1', 'a')); + + expect(current('a'), [_item('1', 'a')]); + }); +} diff --git a/mobile/test/medium/repositories/people_repository_test.dart b/mobile/test/medium/repositories/people_repository_test.dart index 041aa46ea..b5ff62ba8 100644 --- a/mobile/test/medium/repositories/people_repository_test.dart +++ b/mobile/test/medium/repositories/people_repository_test.dart @@ -5,11 +5,11 @@ import '../repository_context.dart'; void main() { late MediumRepositoryContext ctx; - late PeopleRepository sut; + late PeopleDatabaseRepository sut; setUp(() { ctx = MediumRepositoryContext(); - sut = PeopleRepository(ctx.db); + sut = PeopleDatabaseRepository(ctx.db); }); tearDown(() async { diff --git a/mobile/test/services/deep_link_service_test.dart b/mobile/test/services/deep_link_service_test.dart index 2d07042bc..09ea2a19d 100644 --- a/mobile/test/services/deep_link_service_test.dart +++ b/mobile/test/services/deep_link_service_test.dart @@ -8,7 +8,6 @@ import 'package:immich_mobile/domain/models/timeline.model.dart'; import 'package:immich_mobile/domain/models/user.model.dart'; import 'package:immich_mobile/domain/services/asset.service.dart'; import 'package:immich_mobile/domain/services/memory.service.dart'; -import 'package:immich_mobile/domain/services/people.service.dart'; import 'package:immich_mobile/domain/services/remote_album.service.dart'; import 'package:immich_mobile/domain/services/timeline.service.dart'; import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.dart'; @@ -24,7 +23,6 @@ class MockRemoteAlbumService extends Mock implements RemoteAlbumService {} class MockMemoryService extends Mock implements MemoryService {} -class MockPeopleService extends Mock implements PeopleService {} class MockPlatformDeepLink extends Mock implements PlatformDeepLink {} @@ -111,14 +109,7 @@ void main() { when(() => ref.read(assetViewerProvider.notifier)).thenReturn(MockAssetViewerStateNotifier()); - sut = DeepLinkService( - timelineFactory, - assetService, - remoteAlbumService, - memoryService, - MockPeopleService(), - _user, - ); + sut = DeepLinkService(timelineFactory, assetService, remoteAlbumService, memoryService, _user); addTearDown(() async { for (final timelineService in createdTimelineServices) {