diff --git a/mobile/lib/mixins/stream_notifier.mixin.dart b/mobile/lib/mixins/stream_notifier.mixin.dart new file mode 100644 index 0000000000..a068c46346 --- /dev/null +++ b/mobile/lib/mixins/stream_notifier.mixin.dart @@ -0,0 +1,37 @@ +import 'dart:async'; + +import 'package:hooks_riverpod/hooks_riverpod.dart'; + +/// Converts a [Stream] into an [AsyncNotifier] +/// +/// The [build] method MUST call [buildFromStream] in order to properly subscribe +mixin StreamNotifierMixin { + set state(AsyncValue value); + + /// Forwards data into the notifier from [stream]. Future will stay open until [stream] completes + /// + /// Must be called in [build] + Future buildFromStream( + Ref> ref, + Stream stream, { + required T Function(Object error, StackTrace stack) onError, + }) { + final completer = Completer(); + + void apply(T value) { + if (completer.isCompleted) { + state = AsyncData(value); + } else { + completer.complete(value); + } + } + + final subscription = stream.listen( + apply, + onError: (Object error, StackTrace stack) => apply(onError(error, stack)), + ); + ref.onDispose(subscription.cancel); + + return completer.future; + } +} diff --git a/mobile/lib/presentation/pages/drift_activities.page.dart b/mobile/lib/presentation/pages/drift_activities.page.dart index ebf7c2efa7..effed4c3ef 100644 --- a/mobile/lib/presentation/pages/drift_activities.page.dart +++ b/mobile/lib/presentation/pages/drift_activities.page.dart @@ -9,8 +9,8 @@ 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/drift_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/store/activity.dart'; import 'package:immich_mobile/widgets/activities/comment_bubble.dart'; @RoutePage() 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 46a5c2c41e..63c1181854 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 @@ -2,14 +2,14 @@ import 'package:collection/collection.dart'; import 'package:easy_localization/easy_localization.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_data/model/activity.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/extensions/translate_extensions.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'; +import 'package:immich_mobile/store/activity.dart'; class LikeActivityActionButton extends ConsumerWidget { const LikeActivityActionButton({super.key, this.iconOnly = false, this.menuItem = false}); @@ -35,8 +35,6 @@ class LikeActivityActionButton extends ConsumerWidget { } else { await ref.read(albumActivityProvider((album?.id ?? "", asset?.id)).notifier).addLike(); } - - ref.invalidate(albumActivityProvider((album?.id ?? "", asset?.id))); } return activities.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 deae9cbd07..b85b415521 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 @@ -10,13 +10,13 @@ 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'; import 'package:immich_mobile/providers/infrastructure/readonly_mode.provider.dart'; import 'package:immich_mobile/providers/routes.provider.dart'; import 'package:immich_mobile/routing/router.dart'; +import 'package:immich_mobile/store/activity.dart'; import 'package:immich_mobile/utils/timezone.dart'; import 'package:immich_ui/immich_ui.dart'; diff --git a/mobile/lib/providers/activity.provider.dart b/mobile/lib/providers/activity.provider.dart deleted file mode 100644 index b2cdbcf18c..0000000000 --- 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 3be6c6b234..0000000000 --- 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/data_store.dart b/mobile/lib/providers/infrastructure/data_store.dart index 835f7eac2f..05d2190ee6 100644 --- a/mobile/lib/providers/infrastructure/data_store.dart +++ b/mobile/lib/providers/infrastructure/data_store.dart @@ -11,6 +11,8 @@ abstract final class Store { static final people = _store((c) => c.people); + static final activities = _store((c) => c.activities); + /// Direct database access for the repositories that have not yet moved into `immich_data` // TODO(rewrite): Remove this provider once all repositories have migrated to `immich_data` static final db = _store((c) => c.db); diff --git a/mobile/lib/services/activity.service.dart b/mobile/lib/services/activity.service.dart deleted file mode 100644 index 0d4709d0d5..0000000000 --- a/mobile/lib/services/activity.service.dart +++ /dev/null @@ -1,66 +0,0 @@ -import 'package:hooks_riverpod/hooks_riverpod.dart'; -import 'package:immich_mobile/constants/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/store/activity.dart b/mobile/lib/store/activity.dart new file mode 100644 index 0000000000..5c2fe21229 --- /dev/null +++ b/mobile/lib/store/activity.dart @@ -0,0 +1,61 @@ +import 'dart:async'; + +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_data/model/activity.dart'; +import 'package:immich_mobile/mixins/error_logger.mixin.dart'; +import 'package:immich_mobile/mixins/stream_notifier.mixin.dart'; +import 'package:immich_mobile/providers/infrastructure/data_store.dart'; +import 'package:logging/logging.dart'; + +/// Activities associated with a `albumId`, `assetId` pair +final albumActivityProvider = AsyncNotifierProvider.autoDispose + .family<_AlbumActivity, List, (String albumId, String? assetId)>(_AlbumActivity.new); + +class _AlbumActivity extends AutoDisposeFamilyAsyncNotifier, (String albumId, String? assetId)> + with ErrorLoggerMixin, StreamNotifierMixin> { + @override + final Logger logger = Logger("ActivityService"); + + late String albumId; + late String? assetId; + + @override + Future> build((String albumId, String? assetId) args) { + albumId = args.$1; + assetId = args.$2; + + return buildFromStream( + ref, + // TODO(rewrite): `force: true` matches the previous behavior of a HTTP request on mount + ref.watch(Store.activities).getAll(albumId, assetId: assetId, force: true), + onError: (error, stack) { + logger.severe("Failed to get all activities for album $albumId", error, stack); + return const []; + }, + ); + } + + Future removeActivity(String id) async { + await logError( + () async { + await ref.read(Store.activities).remove(albumId, id); + }, + defaultValue: null, + errorMessage: "Failed to delete activity", + ); + } + + Future addLike() async { + await guardError( + () => ref.read(Store.activities).addLike(albumId, assetId: assetId), + errorMessage: "Failed to create like for album $albumId", + ); + } + + Future addComment(String comment) async { + await guardError( + () => ref.read(Store.activities).addComment(albumId, comment, assetId: assetId), + errorMessage: "Failed to create comment for album $albumId", + ); + } +} diff --git a/mobile/lib/utils/provider_utils.dart b/mobile/lib/utils/provider_utils.dart index 05bb729c50..6f944a4440 100644 --- a/mobile/lib/utils/provider_utils.dart +++ b/mobile/lib/utils/provider_utils.dart @@ -1,14 +1,12 @@ import 'package:hooks_riverpod/hooks_riverpod.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/asset_api.repository.dart'; import 'package:immich_mobile/repositories/drift_album_api_repository.dart'; import 'package:immich_mobile/repositories/partner_api.repository.dart'; void invalidateAllApiRepositoryProviders(WidgetRef ref) { ref.invalidate(userApiRepositoryProvider); - ref.invalidate(activityApiRepositoryProvider); ref.invalidate(partnerApiRepositoryProvider); ref.invalidate(assetApiRepositoryProvider); ref.invalidate(searchApiRepositoryProvider); diff --git a/mobile/lib/widgets/activities/comment_bubble.dart b/mobile/lib/widgets/activities/comment_bubble.dart index fcaff8bfc3..94ecaa729c 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_data/model/activity.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/store/activity.dart'; import 'package:immich_mobile/widgets/activities/dismissible_activity.dart'; import 'package:immich_mobile/widgets/common/user_circle_avatar.dart'; @@ -33,15 +37,19 @@ class CommentBubble extends ConsumerWidget { ); 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), + ), + ); } // avatar (hidden for own messages) diff --git a/mobile/packages/data/lib/data_controller.dart b/mobile/packages/data/lib/data_controller.dart index be364be160..461eab254f 100644 --- a/mobile/packages/data/lib/data_controller.dart +++ b/mobile/packages/data/lib/data_controller.dart @@ -2,7 +2,9 @@ import 'package:flutter/foundation.dart'; import 'package:immich_data/db/database.dart'; import 'package:immich_data/db/logger.dart'; import 'package:immich_data/db/person.dart'; +import 'package:immich_data/server/activity.dart'; import 'package:immich_data/server/person.dart'; +import 'package:immich_data/store/activity.dart'; import 'package:immich_data/store/person.dart'; import 'package:openapi/api.dart'; import 'package:sqlite3/common.dart'; @@ -62,6 +64,10 @@ class DataController { PersonApiRepository(PeopleApi(_apiClient)), ); + // This is optional and not lazy so we only call `dispose` if necessary + ActivityService? _activities; + ActivityService get activities => _activities ??= ActivityService(ActivityApiRepository(ActivitiesApi(_apiClient))); + /// Direct database access for the logic that has not yet moved into this package // TODO(rewrite): Remove once all repositories have migrated into this package Drift get db => _db; @@ -71,6 +77,9 @@ class DataController { DriftLogger get logDb => _logDb; Future close() async { + await _activities?.dispose(); + _activities = null; + await _db.close(); // Close after the primary DB to ensure all logs are captured diff --git a/mobile/lib/models/activities/activity.model.dart b/mobile/packages/data/lib/model/activity.dart similarity index 89% rename from mobile/lib/models/activities/activity.model.dart rename to mobile/packages/data/lib/model/activity.dart index d3f99aea64..946c74d4e5 100644 --- a/mobile/lib/models/activities/activity.model.dart +++ b/mobile/packages/data/lib/model/activity.dart @@ -1,4 +1,4 @@ -import 'package:immich_mobile/domain/models/user.model.dart'; +import 'package:immich_data/model/user/user.dart'; enum ActivityType { comment, like } @@ -61,9 +61,3 @@ class Activity { return id.hashCode ^ assetId.hashCode ^ comment.hashCode ^ createdAt.hashCode ^ type.hashCode ^ user.hashCode; } } - -class ActivityStats { - final int comments; - - const ActivityStats({required this.comments}); -} diff --git a/mobile/lib/repositories/activity_api.repository.dart b/mobile/packages/data/lib/server/activity.dart similarity index 61% rename from mobile/lib/repositories/activity_api.repository.dart rename to mobile/packages/data/lib/server/activity.dart index 9ae83b0e7b..6e999a3b96 100644 --- a/mobile/lib/repositories/activity_api.repository.dart +++ b/mobile/packages/data/lib/server/activity.dart @@ -1,18 +1,15 @@ -import 'package:hooks_riverpod/hooks_riverpod.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:immich_data/model/activity.dart'; +import 'package:immich_data/server/api_repository.dart'; +import 'package:immich_data/server/util/convert.dart'; +import 'package:meta/meta.dart'; import 'package:openapi/api.dart'; -final activityApiRepositoryProvider = Provider( - (ref) => ActivityApiRepository(ref.watch(apiServiceProvider).activitiesApi), -); - +/// Immich HTTP API for album activity (comments and likes) class ActivityApiRepository extends ApiRepository { final ActivitiesApi _api; - ActivityApiRepository(this._api); + @internal + const ActivityApiRepository(this._api); Future> getAll(String albumId, {String? assetId}) async { final response = await checkNull(_api.getActivities(albumId, assetId: assetId)); @@ -31,19 +28,15 @@ class ActivityApiRepository extends ApiRepository { } Future delete(String id) { + // TODO(agg23): I think this is a bug; `checkNull` will always throw here return checkNull(_api.deleteActivity(id)); } - Future getStats(String albumId, {String? assetId}) async { - final response = await checkNull(_api.getActivityStatistics(albumId, assetId: assetId)); - return ActivityStats(comments: response.comments); - } - static Activity _toActivity(ActivityResponseDto dto) => Activity( id: dto.id, createdAt: dto.createdAt, type: dto.type == ReactionType.comment ? ActivityType.comment : ActivityType.like, - user: UserConverter.fromSimpleUserDto(dto.user), + user: DtoConverter.toUser(dto.user), assetId: dto.assetId, comment: dto.comment.orElse(null), ); diff --git a/mobile/packages/data/lib/server/util/convert.dart b/mobile/packages/data/lib/server/util/convert.dart new file mode 100644 index 0000000000..49e0d7225d --- /dev/null +++ b/mobile/packages/data/lib/server/util/convert.dart @@ -0,0 +1,29 @@ +import 'package:immich_data/model/user/user.dart'; +import 'package:openapi/api.dart'; + +abstract final class DtoConverter { + // TODO(rewrite): This is duplicated from lib/infrastructure/utils/user.converter.dart + static UserDto toUser(UserResponseDto dto) => UserDto( + id: dto.id, + email: dto.email, + name: dto.name, + isAdmin: false, + updatedAt: DateTime.now(), + hasProfileImage: dto.profileImagePath.isNotEmpty, + profileChangedAt: dto.profileChangedAt, + avatarColor: _toAvatarColor(dto.avatarColor), + ); +} + +AvatarColor _toAvatarColor(UserAvatarColor color) => switch (color) { + UserAvatarColor.red => AvatarColor.red, + UserAvatarColor.green => AvatarColor.green, + UserAvatarColor.blue => AvatarColor.blue, + UserAvatarColor.purple => AvatarColor.purple, + UserAvatarColor.orange => AvatarColor.orange, + UserAvatarColor.pink => AvatarColor.pink, + UserAvatarColor.amber => AvatarColor.amber, + UserAvatarColor.yellow => AvatarColor.yellow, + UserAvatarColor.gray => AvatarColor.gray, + UserAvatarColor.primary || _ => AvatarColor.primary, +}; diff --git a/mobile/packages/data/lib/store/activity.dart b/mobile/packages/data/lib/store/activity.dart new file mode 100644 index 0000000000..71a0573cfd --- /dev/null +++ b/mobile/packages/data/lib/store/activity.dart @@ -0,0 +1,87 @@ +import 'package:immich_data/model/activity.dart'; +import 'package:immich_data/server/activity.dart'; +import 'package:immich_data/server/errors.dart'; +import 'package:immich_data/store/util/stream_cache.dart'; +import 'package:meta/meta.dart'; + +/// Activities (comments and likes) on shared albums and their assets +/// +/// State is fetched over HTTP and mutated and cached in memory +// TODO(agg23): This should not be called simply "Activity" +class ActivityService { + final ActivityApiRepository _api; + + late final StreamCache<(String albumId, String? assetId), List> _cache = StreamCache( + fetch: (scope) => _api.getAll(scope.$1, assetId: scope.$2), + ); + + @internal + ActivityService(this._api); + + /// All activities for an album specified by [albumId], or all activities for a specific asset within that album. + /// Providing [force] will make a new HTTP request on stream open (legacy behavior) + /// + /// **NOTE:** This is currently reactive only to in memory mutation, not live HTTP changes + Stream> getAll(String albumId, {String? assetId, bool force = false}) { + return _cache.get((albumId, assetId), force: force); + } + + /// 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 { + final activity = await _api.create(albumId, ActivityType.comment, assetId: assetId, comment: comment); + _upsert(albumId, activity); + return activity; + } + + /// 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 { + final activity = await _api.create(albumId, ActivityType.like, assetId: assetId); + _upsert(albumId, activity); + return activity; + } + + /// Remove an activity by its [activityId] + Future remove(String albumId, String activityId) async { + try { + await _api.delete(activityId); + } on NoResponseDtoError { + // TODO(agg23): This error should not be thrown at all + } + + // Only drop on "success" (including the broken NoResponseDtoError above) + _drop(albumId, activityId); + } + + /// Terminate all streams and dispose of the cache + Future dispose() { + return _cache.dispose(); + } + + /// Apply an activity upsert to the in memory cache + void _upsert(String albumId, Activity activity) { + _cache.update( + // If there is a list for our album, we update it no matter what + // If there is a list for our specific asset, we also update that + (scope) => scope.$1 == albumId && (scope.$2 == null || scope.$2 == activity.assetId), + (activities) { + final index = activities.indexWhere((a) => a.id == activity.id); + + if (index == -1) { + // Insert new item + return [...activities, activity]; + } else if (activities[index] == activity) { + // No change + return activities; + } else { + // Update existing + return [...activities]..[index] = activity; + } + }, + ); + } + + /// Drop an activity from any cache entry that may contain it + void _drop(String albumId, String activityId) { + _cache.update((scope) => scope.$1 == albumId, (activities) => activities.where((a) => a.id != activityId).toList()); + } +} diff --git a/mobile/packages/data/lib/store/person.dart b/mobile/packages/data/lib/store/person.dart index 59cef4d234..7ec5dfe31a 100644 --- a/mobile/packages/data/lib/store/person.dart +++ b/mobile/packages/data/lib/store/person.dart @@ -3,11 +3,11 @@ import 'package:immich_data/model/person.dart'; import 'package:immich_data/server/person.dart'; import 'package:meta/meta.dart'; +// TODO(rewrite): This needs to be made reactive class PersonService { final PersonDatabaseRepository _db; final PersonApiRepository _api; - /// Constructed by [ImmichData]; the app obtains instances from there. @internal const PersonService(this._db, this._api); diff --git a/mobile/packages/data/lib/store/util/stream_cache.dart b/mobile/packages/data/lib/store/util/stream_cache.dart new file mode 100644 index 0000000000..f3064ccb67 --- /dev/null +++ b/mobile/packages/data/lib/store/util/stream_cache.dart @@ -0,0 +1,144 @@ +import 'dart:async'; + +/// A in memory cache for streams that are backed by emphemeral data (typically HTTP servers). +/// Values are fetched on demand and disposed of when the last subscriber disconnects +class StreamCache { + final Future Function(K key) _fetch; + + final Map> _entries = {}; + final Map> _inflightFetches = {}; + + StreamCache({required this._fetch}); + + /// A live view of the value corresponding to [key]. [force] skips the cached emission and always fetches + // TODO(rewrite): Remove force; it exists only for old Flutter views that expect 0 data on first subscribe + Stream get(K key, {bool force = false}) { + // Reprents this call's subscription + late final StreamController localStreamController; + // Reprents the source of all updates for this key, shared between individual `get` callers + StreamSubscription? cacheStreamController; + + localStreamController = StreamController( + onListen: () { + final entry = _entries.putIfAbsent(key, _CacheEntry.new).ref(); + + // Push stream updates from the upstream to the local controller + cacheStreamController = entry.updateStreamController.stream.listen( + localStreamController.add, + onDone: () => unawaited(localStreamController.close()), + ); + + final currentData = entry.value; + if (currentData != null && !force) { + // If we have a cached value at call time, and we're not using legacy `force` behavior, immediately emit that value + localStreamController.add(currentData); + } + + if (currentData == null || force) { + // No data/force request, request new data from upstream + unawaited( + _fetchDeduped( + key, + ).catchError((Object error, StackTrace stack) => localStreamController.addError(error, stack)), + ); + } + }, + onCancel: () async { + await cacheStreamController?.cancel(); + + final entry = _entries[key]; + if (entry == null) { + return; + } + + if (entry.deref().refCount <= 0) { + _entries.remove(key); + } + }, + ); + + return localStreamController.stream; + } + + /// Apply [transform] to all values corresponding to keys matching [predicate] + /// + /// Returning the same value (identity) from [transform] will not emit a new value + void update(bool Function(K key) predicate, V Function(V value) transform) { + for (final MapEntry(:key, value: entry) in _entries.entries) { + if (!predicate(key)) { + continue; + } + + final currentValue = entry.value; + if (currentValue == null) { + continue; + } + + final newValue = transform(currentValue); + if (!identical(newValue, currentValue)) { + entry.value = newValue; + } + } + } + + /// Close all streams and delete caches + Future dispose() async { + _inflightFetches.clear(); + + final oldEntries = [..._entries.values]; + _entries.clear(); + + await Future.wait(oldEntries.map((entry) => entry.updateStreamController.close())); + } + + /// Fetch a single value matching the provided [key]. If a matching fetch is already in progress, await that fetch + Future _fetchDeduped(K key) { + return _inflightFetches.putIfAbsent(key, () async { + try { + final data = await _fetch(key); + final entry = _entries[key]; + + if (entry != null) { + entry.value = data; + } + } finally { + unawaited(_inflightFetches.remove(key)); + } + }); + } +} + +/// A streaming in memory cache entry +class _CacheEntry { + V? _value; + int refCount = 0; + + final StreamController updateStreamController = StreamController.broadcast(); + + /// The current value in the cache + V? get value => _value; + + /// Set the current cache value, sending it to all subscribers + set value(V newValue) { + _value = newValue; + updateStreamController.add(newValue); + } + + /// Increments the reference count of the entry + _CacheEntry ref() { + refCount += 1; + return this; + } + + /// Decrements the reference count of the entry. If there are no active references, closes the update stream + _CacheEntry deref() { + if (refCount == 0) { + unawaited(updateStreamController.close()); + + return this; + } + + refCount -= 1; + return this; + } +} diff --git a/mobile/packages/data/pubspec.yaml b/mobile/packages/data/pubspec.yaml index 55f3958078..ceb9e608bf 100644 --- a/mobile/packages/data/pubspec.yaml +++ b/mobile/packages/data/pubspec.yaml @@ -27,6 +27,7 @@ dev_dependencies: flutter_lints: ^5.0.0 flutter_test: sdk: flutter + mocktail: ^1.0.5 # The generated openapi client depends back on immich_mobile, so resolving # this package pulls in the app's dependency graph. Pub only honors the root diff --git a/mobile/packages/data/test/store/util/stream_cache_test.dart b/mobile/packages/data/test/store/util/stream_cache_test.dart new file mode 100644 index 0000000000..7a7dac25e5 --- /dev/null +++ b/mobile/packages/data/test/store/util/stream_cache_test.dart @@ -0,0 +1,148 @@ +import 'dart:async'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_data/store/util/stream_cache.dart'; + +void main() { + late int fetches; + late StreamCache> cache; + + setUp(() { + fetches = 0; + cache = StreamCache(fetch: (key) async => ['$key-v${++fetches}']); + addTearDown(cache.dispose); + }); + + /// Subscribes and collects every emission + List> subscribe(String key, {bool force = false}) { + final emissions = >[]; + final subscription = cache.get(key, force: force).listen(emissions.add); + addTearDown(subscription.cancel); + return emissions; + } + + test('fetches and emits on first subscribe', () async { + await expectLater(cache.get('a'), emits(['a-v1'])); + }); + + test('a second subscriber is served from the cache without a fetch', () async { + subscribe('a'); + await pumpEventQueue(); + + await expectLater(cache.get('a'), emits(['a-v1'])); + + expect(fetches, 1); + }); + + test('force skips the cached emission and always fetches', () async { + subscribe('a'); + await pumpEventQueue(); + + await expectLater(cache.get('a', force: true), emits(['a-v2'])); + + expect(fetches, 2); + }); + + test('keys are cached independently', () async { + await expectLater(cache.get('a'), emits(['a-v1'])); + await expectLater(cache.get('b'), emits(['b-v2'])); + }); + + test('concurrent first subscribers share a single fetch', () async { + final completer = Completer>(); + cache = StreamCache( + fetch: (key) { + fetches++; + return completer.future; + }, + ); + addTearDown(cache.dispose); + final first = subscribe('a'); + final second = subscribe('a'); + + completer.complete(['shared']); + await pumpEventQueue(); + + expect(first, [ + ['shared'], + ]); + expect(first, second); + expect(fetches, 1); + }); + + test('a failed fetch is a stream error and the stream survives', () async { + var fail = true; + cache = StreamCache(fetch: (key) => fail ? Future.error(StateError('down')) : Future.value(['recovered'])); + addTearDown(cache.dispose); + + final events = []; + final subscription = cache.get('a').listen(events.add, onError: (Object error) => events.add('error')); + addTearDown(subscription.cancel); + await pumpEventQueue(); + expect(events, ['error']); + + fail = false; + subscribe('a', force: true); + await pumpEventQueue(); + + expect(events, [ + 'error', + ['recovered'], + ]); + }); + + test('the cache is dropped when the last subscriber cancels', () async { + final subscription = cache.get('a').listen((_) {}); + await pumpEventQueue(); + await subscription.cancel(); + + await expectLater(cache.get('a'), emits(['a-v2'])); + + expect(fetches, 2); + }); + + group('update', () { + test('transforms and publishes every matching cached value', () async { + final a = subscribe('a'); + final b = subscribe('b'); + final c = subscribe('c'); + await pumpEventQueue(); + + cache.update((key) => key != 'c', (value) => [...value, 'patched']); + await pumpEventQueue(); + + expect(a.last, ['a-v1', 'patched']); + expect(b.last, ['b-v2', 'patched']); + expect(c.last, ['c-v3']); + }); + + test('publishes nothing when the transform returns the identical value', () async { + final a = subscribe('a'); + await pumpEventQueue(); + + cache.update((_) => true, (value) => value); + await pumpEventQueue(); + + expect(a, hasLength(1)); + }); + + test('does not touch keys that have no cached value yet', () { + cache.update((_) => true, (value) => [...value, 'patched']); + // No cached values exist; nothing to transform and nothing thrown. + expect(fetches, 0); + }); + }); + + test('dispose ends every subscriber stream', () async { + var done = false; + final subscription = cache.get('a').listen((_) {}); + subscription.onDone(() => done = true); + addTearDown(subscription.cancel); + await pumpEventQueue(); + + await cache.dispose(); + await pumpEventQueue(); + + expect(done, isTrue); + }); +} diff --git a/mobile/test/providers/activity_provider_test.dart b/mobile/test/providers/activity_provider_test.dart new file mode 100644 index 0000000000..c2e8c99207 --- /dev/null +++ b/mobile/test/providers/activity_provider_test.dart @@ -0,0 +1,144 @@ +import 'dart:async'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_data/data_controller.dart'; +import 'package:immich_data/model/activity.dart'; +import 'package:immich_data/model/user/user.dart'; +import 'package:immich_data/store/activity.dart'; +import 'package:immich_mobile/providers/infrastructure/data_store.dart'; +import 'package:immich_mobile/store/activity.dart'; +import 'package:mocktail/mocktail.dart'; + +class MockDataController extends Mock implements DataController {} + +class MockActivityService extends Mock implements ActivityService {} + +void main() { + late ProviderContainer container; + late MockActivityService service; + late StreamController> albumScopedStream; + late StreamController> assetScopedStream; + + const albumId = 'album-1'; + const assetId = 'asset-1'; + const albumScoped = (albumId, null); + const assetScoped = (albumId, assetId); + + final user = UserDto(id: 'user-1', email: 'user@test.com', name: 'User', profileChangedAt: DateTime.utc(2025)); + + Activity activity(String id, {String? assetId, ActivityType type = ActivityType.comment, String? comment}) => + Activity(id: id, assetId: assetId, comment: comment, createdAt: DateTime.utc(2025), type: type, user: user); + + Future pumpBothScopes() async { + container.listen(albumActivityProvider(assetScoped), (_, _) {}); + container.listen(albumActivityProvider(albumScoped), (_, _) {}); + albumScopedStream.add([]); + assetScopedStream.add([]); + await container.read(albumActivityProvider(assetScoped).future); + await container.read(albumActivityProvider(albumScoped).future); + } + + List idsIn((String, String?) scope) => + container.read(albumActivityProvider(scope)).requireValue.map((a) => a.id).toList(); + + setUp(() { + service = MockActivityService(); + albumScopedStream = StreamController>.broadcast(); + assetScopedStream = StreamController>.broadcast(); + addTearDown(albumScopedStream.close); + addTearDown(assetScopedStream.close); + when(() => service.getAll(albumId, assetId: null, force: true)).thenAnswer((_) => albumScopedStream.stream); + when(() => service.getAll(albumId, assetId: assetId, force: true)).thenAnswer((_) => assetScopedStream.stream); + + final controller = MockDataController(); + when(() => controller.activities).thenReturn(service); + + container = ProviderContainer(overrides: [Store.overrideWithValue(controller)]); + addTearDown(container.dispose); + }); + + group('build', () { + test('album/asset scopes get their own views', () async { + container.listen(albumActivityProvider(assetScoped), (_, _) {}); + container.listen(albumActivityProvider(albumScoped), (_, _) {}); + albumScopedStream.add([activity('c1'), activity('c2', assetId: assetId)]); + assetScopedStream.add([activity('c2', assetId: assetId)]); + + expect(await container.read(albumActivityProvider(albumScoped).future), hasLength(2)); + expect(await container.read(albumActivityProvider(assetScoped).future), hasLength(1)); + }); + + test('errors become an empty list', () async { + container.listen(albumActivityProvider(albumScoped), (_, _) {}); + albumScopedStream.addError(Exception('network down')); + + expect(await container.read(albumActivityProvider(albumScoped).future), isEmpty); + }); + }); + + test('new events get pushed to Riverpod', () async { + await pumpBothScopes(); + + final like = activity('l1', assetId: assetId, type: ActivityType.like); + albumScopedStream.add([like]); + assetScopedStream.add([like]); + await pumpEventQueue(); + + expect(idsIn(albumScoped), ['l1']); + expect(idsIn(assetScoped), ['l1']); + }); + + group('mutations', () { + test('addLike calls API', () async { + final like = activity('l1', assetId: assetId, type: ActivityType.like); + when(() => service.addLike(albumId, assetId: assetId)).thenAnswer((_) async => like); + await pumpBothScopes(); + + await container.read(albumActivityProvider(assetScoped).notifier).addLike(); + + verify(() => service.addLike(albumId, assetId: assetId)).called(1); + }); + + test('addComment calls API', () async { + final comment = activity('c1', assetId: assetId, comment: 'nice'); + when(() => service.addComment(albumId, 'nice', assetId: assetId)).thenAnswer((_) async => comment); + await pumpBothScopes(); + + await container.read(albumActivityProvider(assetScoped).notifier).addComment('nice'); + + verify(() => service.addComment(albumId, 'nice', assetId: assetId)).called(1); + }); + + test('removeActivity calls API', () async { + when(() => service.remove(albumId, 'c1')).thenAnswer((_) async {}); + await pumpBothScopes(); + + await container.read(albumActivityProvider(assetScoped).notifier).removeActivity('c1'); + + verify(() => service.remove(albumId, 'c1')).called(1); + }); + }); + + group('failed mutations', () { + test('addLike logs error', () async { + when(() => service.addLike(albumId, assetId: assetId)).thenAnswer((_) => Future.error(Exception('rejected'))); + await pumpBothScopes(); + + await container.read(albumActivityProvider(assetScoped).notifier).addLike(); + + expect(idsIn(assetScoped), isEmpty); + expect(idsIn(albumScoped), isEmpty); + }); + + test('removeActivity logs error', () async { + when(() => service.remove(albumId, 'c1')).thenAnswer((_) => Future.error(Exception('rejected'))); + await pumpBothScopes(); + + await container.read(albumActivityProvider(assetScoped).notifier).removeActivity('c1'); + + expect(idsIn(assetScoped), isEmpty); + expect(idsIn(albumScoped), isEmpty); + }); + }); +}