chore(mobile): make Person Store reactive via Drift (#31661)

mobile/lib/data/db/main/dao/person.dart

@@ -11,14 +11,15 @@ class PeopleDatabaseRepository extends DatabaseAccessor<Drift> with $PeopleDatab
 
   Drift get _db => attachedDatabase;
 
-  Future<Person?> get(String personId) async {
+  /// The person for the given [personId], if any
+  Stream<Person?> watchPerson(String personId) {
     final query = _db.select(_db.personEntity)..where((row) => row.id.equals(personId));
 
-    final result = await query.getSingleOrNull();
-    return result?.toDto();
+    return query.map((row) => row.toDto()).watchSingleOrNull();
   }
 
-  Future<List<Person>> getAssetPeople(String assetId) async {
+  /// The people associated with a given [assetId]
+  Stream<List<Person>> watchPeopleForAsset(String assetId) {
     // An asset can have multiple face records for the same person (e.g., metadata
     // imports alongside ML detections). Use a subquery instead of a join so each
     // person is returned once, regardless of how many of their faces are on the asset
@@ -33,10 +34,13 @@ class PeopleDatabaseRepository extends DatabaseAccessor<Drift> with $PeopleDatab
     final query = _db.select(_db.personEntity)
       ..where((row) => row.id.isInQuery(faceQuery) & row.isHidden.equals(false));
 
-    return query.map((row) => row.toDto()).get();
+    return query.map((row) => row.toDto()).watch();
   }
 
-  Stream<List<Person>> watch({int minFaces = 3}) {
+  /// All known people with a known associated face and asset
+  ///
+  /// If [minFaces] is provided (defaults to 3), restrict to people having at least that many unique face entries
+  Stream<List<Person>> watchAll({int minFaces = 3}) {
     final people = _db.personEntity;
     final faces = _db.assetFaceEntity;
     final assets = _db.remoteAssetEntity;

mobile/lib/data/store/person.dart

@@ -1,3 +1,4 @@
+import 'package:collection/collection.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
 import 'package:immich_mobile/data/server/person.dart';
 import 'package:immich_mobile/data/store/util/cache.dart';
@@ -15,13 +16,13 @@ extension type const PersonStore._(Provider<PersonMutations> _provider) implemen
 
   /// Get the person specified by [personId]
   ///
-  /// **NOTE:** This is not reactive to changes, and only hits the local DB
-  AutoDisposeFutureProvider<Person?> byId(String personId) => _byIdProvider(personId);
+  /// **NOTE:** This is only reactive to the local DB
+  AutoDisposeStreamProvider<Person?> 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<List<Person>> forAsset(String assetId) => _forAssetProvider(assetId);
+  /// **NOTE:** This is only reactive to the local DB
+  AutoDisposeStreamProvider<List<Person>> forAsset(String assetId) => _forAssetProvider(assetId);
 
   /// Get all known people, honoring the user's minimum detected face count preference
   ///
@@ -31,19 +32,21 @@ extension type const PersonStore._(Provider<PersonMutations> _provider) implemen
 
 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<Person?, String>(
-  (ref, personId) => ref.watch(_peopleDb).get(personId),
+final _byIdProvider = StreamProvider.autoDispose.family<Person?, String>(
+  (ref, personId) => ref.watch(_peopleDb).watchPerson(personId).distinct(),
 );
 
-final _forAssetProvider = FutureProvider.autoDispose.family<List<Person>, String>(
-  (ref, assetId) => ref.watch(_peopleDb).getAssetPeople(assetId),
+final _forAssetProvider = StreamProvider.autoDispose.family<List<Person>, String>(
+  (ref, assetId) => ref.watch(_peopleDb).watchPeopleForAsset(assetId).distinct(const ListEquality<Person>().equals),
 );
 
 final _allProvider = StreamProvider.autoDispose<List<Person>>((ref) async* {
   final prefs = await ref.watch(userMetadataPreferencesProvider.future);
-  yield* ref.watch(_peopleDb).watch(minFaces: prefs?.minimumFaces ?? 3);
+
+  yield* ref
+      .watch(_peopleDb)
+      .watchAll(minFaces: prefs?.minimumFaces ?? 3)
+      .distinct(const ListEquality<Person>().equals);
 });
 
 class PersonMutations extends StoreMutations {

mobile/lib/presentation/widgets/asset_viewer/asset_details/people_details.widget.dart

@@ -28,22 +28,9 @@ class PeopleDetails extends ConsumerWidget {
       return const SizedBox.shrink();
     }
 
-    final peopleFuture = ref.watch(Store.people.forAsset(asset.id));
+    final people = ref.watch(Store.people.forAsset(asset.id));
 
-    Future<void> showNameEditModal(Person person) async {
-      await showDialog(
-        context: context,
-        useRootNavigator: false,
-        builder: (BuildContext context) {
-          return PersonNameEditForm(person: person);
-        },
-      );
-
-      // TODO(agg23): Remove once state is properly reactive
-      ref.invalidate(Store.people.forAsset(asset.id));
-    }
-
-    return peopleFuture.when(
+    return people.when(
       data: (people) {
         return AnimatedCrossFade(
           firstChild: const SizedBox.shrink(),
@@ -79,7 +66,13 @@ class PeopleDetails extends ConsumerWidget {
                           ContextHelper(context).pop();
                           unawaited(context.pushRoute(PersonRoute(person: person)));
                         },
-                        onNameTap: () => showNameEditModal(person),
+                        onNameTap: () => showDialog(
+                          context: context,
+                          useRootNavigator: false,
+                          builder: (BuildContext context) {
+                            return PersonNameEditForm(person: person);
+                          },
+                        ),
                       ),
                   ],
                 ),

mobile/test/data/store/person_store_test.dart

@@ -1,3 +1,5 @@
+import 'dart:async';
+
 import 'package:flutter_test/flutter_test.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
 import 'package:immich_mobile/data/server/person.dart';
@@ -58,6 +60,48 @@ void main() {
     expect(people.map((p) => p.id), [named.id]);
   });
 
+  test('forAsset re-emits when a person is renamed', () async {
+    final user = await ctx.newUser();
+    final asset = await ctx.newRemoteAsset(ownerId: user.id);
+    final person = await ctx.newPerson(ownerId: user.id, name: 'Old');
+    await ctx.newFace(assetId: asset.id, personId: person.id);
+
+    when(() => api.update(person.id, name: 'New')).thenAnswer((_) async => Person(id: person.id, name: 'New'));
+
+    final renamed = Completer<void>();
+
+    final provider = Store.people.forAsset(asset.id);
+    container.listen(provider, (_, next) {
+      if (next.valueOrNull?.single.name == 'New' && !renamed.isCompleted) {
+        renamed.complete();
+      }
+    });
+    await container.read(provider.future);
+
+    await container.read(Store.people).updateName(person.id, 'New');
+
+    await expectLater(renamed.future, completes);
+  });
+
+  test('forAsset does not push updates unnecessarily', () async {
+    final user = await ctx.newUser();
+    final asset = await ctx.newRemoteAsset(ownerId: user.id);
+    final person = await ctx.newPerson(ownerId: user.id, name: 'Old');
+    await ctx.newFace(assetId: asset.id, personId: person.id);
+
+    var emissions = 0;
+
+    final provider = Store.people.forAsset(asset.id);
+    container.listen(provider, (_, _) => emissions += 1);
+    await container.read(provider.future);
+
+    await ctx.newPerson(ownerId: user.id, name: 'Unrelated');
+
+    await pumpEventQueue();
+    // Only should have received the first, loaded event
+    expect(emissions, 1);
+  });
+
   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');

mobile/test/medium/repositories/people_repository_test.dart

@@ -16,7 +16,7 @@ void main() {
     await ctx.dispose();
   });
 
-  group('getAssetPeople', () {
+  group('watchPeopleForAsset', () {
     test('does not duplicate a person with multiple face records on the same asset', () async {
       // Regression check for #20585: a join on asset_face_entity returned one row
       // per face, so a person appeared twice in the asset details panel when the
@@ -28,7 +28,7 @@ void main() {
       await ctx.newFace(assetId: asset.id, personId: person.id);
       await ctx.newFace(assetId: asset.id, personId: person.id);
 
-      final people = await sut.getAssetPeople(asset.id);
+      final people = await sut.watchPeopleForAsset(asset.id).first;
 
       expect(people, hasLength(1));
       expect(people.single.id, person.id);
@@ -43,7 +43,7 @@ void main() {
       await ctx.newFace(assetId: asset.id, personId: person1.id);
       await ctx.newFace(assetId: asset.id, personId: person2.id);
 
-      final people = await sut.getAssetPeople(asset.id);
+      final people = await sut.watchPeopleForAsset(asset.id).first;
 
       expect(people, hasLength(2));
       expect(people.map((person) => person.id), containsAll([person1.id, person2.id]));
@@ -56,7 +56,7 @@ void main() {
       final hidden = await ctx.newPerson(ownerId: user.id, isHidden: true);
       await ctx.newFace(assetId: asset.id, personId: hidden.id);
 
-      final people = await sut.getAssetPeople(asset.id);
+      final people = await sut.watchPeopleForAsset(asset.id).first;
 
       expect(people, isEmpty);
     });
@@ -69,7 +69,7 @@ void main() {
       final person = await ctx.newPerson(ownerId: user.id);
       await ctx.newFace(assetId: otherAsset.id, personId: person.id);
 
-      final people = await sut.getAssetPeople(asset.id);
+      final people = await sut.watchPeopleForAsset(asset.id).first;
 
       expect(people, isEmpty);
     });