refactor: logger service and remove dynamic

mobile/lib/domain/interfaces/store.interface.dart

@@ -6,9 +6,11 @@ abstract interface class IStoreRepository implements IDatabaseRepository {
 
   Future<T?> tryGet<T>(StoreKey<T> key);
 
+  Future<List<StoreDto<Object>>> getAll();
+
   Stream<T?> watch<T>(StoreKey<T> key);
 
-  Stream<StoreUpdateEvent> watchAll();
+  Stream<StoreDto<Object>> watchAll();
 
   Future<bool> update<T>(StoreKey<T> key, T value);
 

mobile/lib/domain/models/store.model.dart

@@ -75,23 +75,23 @@ enum StoreKey<T> {
   Type get type => T;
 }
 
-class StoreUpdateEvent<T> {
+class StoreDto<T> {
   final StoreKey<T> key;
   final T? value;
 
-  const StoreUpdateEvent(this.key, this.value);
+  const StoreDto(this.key, this.value);
 
   @override
   String toString() {
     return '''
-StoreUpdateEvent: {
+StoreDto: {
   key: $key,
   value: ${value ?? '<NA>'},
 }''';
   }
 
   @override
-  bool operator ==(covariant StoreUpdateEvent<T> other) {
+  bool operator ==(covariant StoreDto<T> other) {
     if (identical(this, other)) return true;
 
     return other.key == key && other.value == value;

mobile/lib/domain/models/sync_event.model.dart

@@ -2,8 +2,7 @@ import 'package:openapi/api.dart';
 
 class SyncEvent {
   final SyncEntityType type;
-  // ignore: avoid-dynamic
-  final dynamic data;
+  final Object data;
   final String ack;
 
   const SyncEvent({required this.type, required this.data, required this.ack});

mobile/lib/domain/services/log.service.dart

@@ -8,6 +8,11 @@ import 'package:immich_mobile/domain/models/log.model.dart';
 import 'package:immich_mobile/domain/models/store.model.dart';
 import 'package:logging/logging.dart';
 
+/// Service responsible for handling application logging.
+///
+/// It listens to Dart's [Logger.root], buffers logs in memory (optionally),
+/// writes them to a persistent [ILogRepository], and manages log levels
+/// via [IStoreRepository]
 class LogService {
   final ILogRepository _logRepository;
   final IStoreRepository _storeRepository;
@@ -18,19 +23,11 @@ class LogService {
   /// This is useful when logging in quick succession, as it increases performance
   /// and reduces NAND wear. However, it may cause the logs to be lost in case of a crash / in isolates.
   final bool _shouldBuffer;
+
   Timer? _flushTimer;
 
   late final StreamSubscription<LogRecord> _logSubscription;
 
-  LogService._(
-    this._logRepository,
-    this._storeRepository,
-    this._shouldBuffer,
-  ) {
-    // Listen to log messages and write them to the database
-    _logSubscription = Logger.root.onRecord.listen(_writeLogToDatabase);
-  }
-
   static LogService? _instance;
   static LogService get I {
     if (_instance == null) {
@@ -44,10 +41,7 @@ class LogService {
     required IStoreRepository storeRepository,
     bool shouldBuffer = true,
   }) async {
-    if (_instance != null) {
-      return _instance!;
-    }
-    _instance = await create(
+    _instance ??= await create(
       logRepository: logRepository,
       storeRepository: storeRepository,
       shouldBuffer: shouldBuffer,
@@ -61,55 +55,28 @@ class LogService {
     bool shouldBuffer = true,
   }) async {
     final instance = LogService._(logRepository, storeRepository, shouldBuffer);
-    // Truncate logs to 250
     await logRepository.truncate(limit: kLogTruncateLimit);
-    // Get log level from store
-    final level = await instance._storeRepository.tryGet(StoreKey.logLevel);
-    if (level != null) {
-      Logger.root.level = Level.LEVELS.elementAtOrNull(level) ?? Level.INFO;
-    }
+    final level = await instance._storeRepository.tryGet(StoreKey.logLevel) ??
+        LogLevel.info.index;
+    Logger.root.level = Level.LEVELS.elementAtOrNull(level) ?? Level.INFO;
     return instance;
   }
 
-  Future<void> setlogLevel(LogLevel level) async {
-    await _storeRepository.insert(StoreKey.logLevel, level.index);
-    Logger.root.level = level.toLevel();
+  LogService._(
+    this._logRepository,
+    this._storeRepository,
+    this._shouldBuffer,
+  ) {
+    _logSubscription = Logger.root.onRecord.listen(_handleLogRecord);
   }
 
-  Future<List<LogMessage>> getMessages() async {
-    final logsFromDb = await _logRepository.getAll();
-    if (_msgBuffer.isNotEmpty) {
-      return [..._msgBuffer.reversed, ...logsFromDb];
-    }
-    return logsFromDb;
-  }
-
-  Future<void> clearLogs() async {
-    _flushTimer?.cancel();
-    _flushTimer = null;
-    _msgBuffer.clear();
-    await _logRepository.deleteAll();
-  }
-
-  /// Flush pending log messages to persistent storage
-  void flush() {
-    if (_flushTimer == null) {
-      return;
-    }
-    _flushTimer!.cancel();
-    // TODO: Rename enable this after moving to sqlite - #16504
-    // await _flushBufferToDatabase();
-  }
-
-  Future<void> dispose() {
-    _flushTimer?.cancel();
-    _logSubscription.cancel();
-    return _flushBufferToDatabase();
-  }
-
-  void _writeLogToDatabase(LogRecord r) {
+  void _handleLogRecord(LogRecord r) {
     if (kDebugMode) {
-      debugPrint('[${r.level.name}] [${r.time}] ${r.message}');
+      debugPrint(
+        '[${r.level.name}] [${r.time}] [${r.loggerName}] ${r.message}'
+        '${r.error == null ? '' : '\nError: ${r.error}'}'
+        '${r.stackTrace == null ? '' : '\nStack: ${r.stackTrace}'}',
+      );
     }
 
     final record = LogMessage(
@@ -125,14 +92,44 @@ class LogService {
       _msgBuffer.add(record);
       _flushTimer ??= Timer(
         const Duration(seconds: 5),
-        () => unawaited(_flushBufferToDatabase()),
+        () => unawaited(flushBuffer()),
       );
     } else {
       unawaited(_logRepository.insert(record));
     }
   }
 
-  Future<void> _flushBufferToDatabase() async {
+  Future<void> setLogLevel(LogLevel level) async {
+    await _storeRepository.insert(StoreKey.logLevel, level.index);
+    Logger.root.level = level.toLevel();
+  }
+
+  Future<List<LogMessage>> getMessages() async {
+    final logsFromDb = await _logRepository.getAll();
+    return [..._msgBuffer.reversed, ...logsFromDb];
+  }
+
+  Future<void> clearLogs() async {
+    _flushTimer?.cancel();
+    _flushTimer = null;
+    _msgBuffer.clear();
+    await _logRepository.deleteAll();
+  }
+
+  void flush() {
+    _flushTimer?.cancel();
+    // TODO: Rename enable this after moving to sqlite - #16504
+    // await _flushBufferToDatabase();
+  }
+
+  Future<void> dispose() {
+    _flushTimer?.cancel();
+    _logSubscription.cancel();
+    return flushBuffer();
+  }
+
+  // TOOD: Move this to private once Isar is removed
+  Future<void> flushBuffer() async {
     _flushTimer = null;
     final buffer = [..._msgBuffer];
     _msgBuffer.clear();

mobile/lib/domain/services/store.service.dart

@@ -3,15 +3,17 @@ import 'dart:async';
 import 'package:immich_mobile/domain/interfaces/store.interface.dart';
 import 'package:immich_mobile/domain/models/store.model.dart';
 
+/// Provides access to a persistent key-value store with an in-memory cache.
+/// Listens for repository changes to keep the cache updated.
 class StoreService {
   final IStoreRepository _storeRepository;
 
-  final Map<int, dynamic> _cache = {};
-  late final StreamSubscription<StoreUpdateEvent> _storeUpdateSubscription;
+  /// In-memory cache. Keys are [StoreKey.id]
+  final Map<int, Object?> _cache = {};
+  late final StreamSubscription<StoreDto> _storeUpdateSubscription;
 
-  StoreService._({
-    required IStoreRepository storeRepository,
-  }) : _storeRepository = storeRepository;
+  StoreService._({required IStoreRepository storeRepository})
+      : _storeRepository = storeRepository;
 
   // TODO: Temporary typedef to make minimal changes. Remove this and make the presentation layer access store through a provider
   static StoreService? _instance;
@@ -23,7 +25,6 @@ class StoreService {
   }
 
   // TODO: Replace the implementation with the one from create after removing the typedef
-  /// Initializes the store with the given [storeRepository]
   static Future<StoreService> init({
     required IStoreRepository storeRepository,
   }) async {
@@ -31,7 +32,6 @@ class StoreService {
     return _instance!;
   }
 
-  /// Initializes the store with the given [storeRepository]
   static Future<StoreService> create({
     required IStoreRepository storeRepository,
   }) async {
@@ -41,16 +41,14 @@ class StoreService {
     return instance;
   }
 
-  /// Fills the cache with the values from the DB
   Future<void> _populateCache() async {
-    for (StoreKey key in StoreKey.values) {
-      final storeValue = await _storeRepository.tryGet(key);
-      _cache[key.id] = storeValue;
+    final storeValues = await _storeRepository.getAll();
+    for (StoreDto storeValue in storeValues) {
+      _cache[storeValue.key.id] = storeValue.value;
     }
   }
 
-  /// Listens for changes in the DB and updates the cache
-  StreamSubscription<StoreUpdateEvent> _listenForChange() =>
+  StreamSubscription<StoreDto> _listenForChange() =>
       _storeRepository.watchAll().listen((event) {
         _cache[event.key.id] = event.value;
       });
@@ -61,11 +59,11 @@ class StoreService {
     _cache.clear();
   }
 
-  /// Returns the stored value for the given key (possibly null)
-  T? tryGet<T>(StoreKey<T> key) => _cache[key.id];
+  /// Returns the cached value for [key], or `null`
+  T? tryGet<T>(StoreKey<T> key) => _cache[key.id] as T?;
 
-  /// Returns the stored value for the given key or if null the [defaultValue]
-  /// Throws a [StoreKeyNotFoundException] if both are null
+  /// Returns the stored value for [key] or [defaultValue].
+  /// Throws [StoreKeyNotFoundException] if value and [defaultValue] are null.
   T get<T>(StoreKey<T> key, [T? defaultValue]) {
     final value = tryGet(key) ?? defaultValue;
     if (value == null) {
@@ -74,23 +72,23 @@ class StoreService {
     return value;
   }
 
-  /// Asynchronously stores the value in the Store
+  /// Stores the [value] for the [key]. Skips write if value hasn't changed.
   Future<void> put<U extends StoreKey<T>, T>(U key, T value) async {
     if (_cache[key.id] == value) return;
     await _storeRepository.insert(key, value);
     _cache[key.id] = value;
   }
 
-  /// Watches a specific key for changes
+  /// Returns a stream that emits the value for [key] on change.
   Stream<T?> watch<T>(StoreKey<T> key) => _storeRepository.watch(key);
 
-  /// Removes the value asynchronously from the Store
+  /// Removes the value for [key]
   Future<void> delete<T>(StoreKey<T> key) async {
     await _storeRepository.delete(key);
     _cache.remove(key.id);
   }
 
-  /// Clears all values from this store (cache and DB)
+  /// Clears all values from thw store (cache and DB)
   Future<void> clear() async {
     await _storeRepository.deleteAll();
     _cache.clear();

mobile/lib/domain/services/sync_stream.service.dart

@@ -1,5 +1,3 @@
-// ignore_for_file: avoid-passing-async-when-sync-expected
-
 import 'dart:async';
 
 import 'package:immich_mobile/domain/interfaces/sync_api.interface.dart';
@@ -59,8 +57,7 @@ class SyncStreamService {
 
   Future<void> _handleSyncData(
     SyncEntityType type,
-    // ignore: avoid-dynamic
-    Iterable<dynamic> data,
+    Iterable<Object> data,
   ) async {
     _logger.fine("Processing sync data for $type of length ${data.length}");
     // ignore: prefer-switch-expression

mobile/lib/domain/utils/background_sync.dart

@@ -1,5 +1,3 @@
-// ignore_for_file: avoid-passing-async-when-sync-expected
-
 import 'dart:async';
 
 import 'package:immich_mobile/providers/infrastructure/sync_stream.provider.dart';
@@ -31,9 +29,9 @@ class BackgroundSyncManager {
     _syncTask = runInIsolateGentle(
       computation: (ref) => ref.read(syncStreamServiceProvider).sync(),
     );
-    _syncTask!.whenComplete(() {
+
+    return _syncTask!.whenComplete(() {
       _syncTask = null;
     });
-    return _syncTask!.future;
   }
 }

mobile/lib/infrastructure/repositories/store.repository.dart

@@ -22,7 +22,7 @@ class IsarStoreRepository extends IsarDatabaseRepository
   }
 
   @override
-  Stream<StoreUpdateEvent> watchAll() {
+  Stream<StoreDto<Object>> watchAll() {
     return _db.storeValues
         .filter()
         .anyOf(validStoreKeys, (query, id) => query.idEqualTo(id))
@@ -71,10 +71,11 @@ class IsarStoreRepository extends IsarDatabaseRepository
         .asyncMap((e) async => e == null ? null : await _toValue(key, e));
   }
 
-  Future<StoreUpdateEvent> _toUpdateEvent(StoreValue entity) async {
-    final key = StoreKey.values.firstWhere((e) => e.id == entity.id);
+  Future<StoreDto<Object>> _toUpdateEvent(StoreValue entity) async {
+    final key = StoreKey.values.firstWhere((e) => e.id == entity.id)
+        as StoreKey<Object>;
     final value = await _toValue(key, entity);
-    return StoreUpdateEvent(key, value);
+    return StoreDto(key, value);
   }
 
   Future<T?> _toValue<T>(StoreKey<T> key, StoreValue entity) async =>
@@ -107,4 +108,13 @@ class IsarStoreRepository extends IsarDatabaseRepository
     };
     return StoreValue(key.id, intValue: intValue, strValue: strValue);
   }
+
+  @override
+  Future<List<StoreDto<Object>>> getAll() async {
+    final entities = await _db.storeValues
+        .filter()
+        .anyOf(validStoreKeys, (query, id) => query.idEqualTo(id))
+        .findAll();
+    return Future.wait(entities.map((e) => _toUpdateEvent(e)).toList());
+  }
 }

mobile/lib/infrastructure/repositories/sync_api.repository.dart

@@ -25,7 +25,6 @@ class SyncApiRepository implements ISyncApiRepository {
     int batchSize = kSyncEventBatchSize,
     http.Client? httpClient,
   }) async {
-    // ignore: avoid-unused-assignment
     final stopwatch = Stopwatch()..start();
     final client = httpClient ?? http.Client();
     final endpoint = "${_api.apiClient.basePath}/sync/stream";
@@ -65,8 +64,7 @@ class SyncApiRepository implements ISyncApiRepository {
     }
 
     try {
-      final response =
-          await client.send(request).timeout(const Duration(seconds: 20));
+      final response = await client.send(request);
 
       if (response.statusCode != 200) {
         final errorBody = await response.stream.bytesToString();
@@ -133,8 +131,7 @@ class SyncApiRepository implements ISyncApiRepository {
   }
 }
 
-// ignore: avoid-dynamic
-const _kResponseMap = <SyncEntityType, Function(dynamic)>{
+const _kResponseMap = <SyncEntityType, Function(Object)>{
   SyncEntityType.userV1: SyncUserV1.fromJson,
   SyncEntityType.userDeleteV1: SyncUserDeleteV1.fromJson,
   SyncEntityType.partnerV1: SyncPartnerV1.fromJson,

mobile/lib/utils/isolate.dart

@@ -3,6 +3,7 @@ import 'dart:ui';
 
 import 'package:flutter/services.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/domain/services/log.service.dart';
 import 'package:immich_mobile/providers/db.provider.dart';
 import 'package:immich_mobile/providers/infrastructure/cancel.provider.dart';
 import 'package:immich_mobile/providers/infrastructure/db.provider.dart';
@@ -58,9 +59,7 @@ Cancelable<T?> runInIsolateGentle<T>({
         stack,
       );
     } finally {
-      // Wait for the logs to flush
-      await Future.delayed(const Duration(seconds: 2));
-      // Always close the new db connection on Isolate end
+      await LogService.I.flushBuffer();
       ref.read(driftProvider).close();
       ref.read(isarProvider).close();
     }

mobile/lib/widgets/settings/advanced_settings.dart

@@ -41,7 +41,7 @@ class AdvancedSettings extends HookConsumerWidget {
     useValueChanged(
       levelId.value,
       (_, __) =>
-          LogService.I.setlogLevel(Level.LEVELS[levelId.value].toLogLevel()),
+          LogService.I.setLogLevel(Level.LEVELS[levelId.value].toLogLevel()),
     );
 
     Future<bool> checkAndroidVersion() async {

mobile/test/domain/services/log_service_test.dart

@@ -74,7 +74,7 @@ void main() {
     setUp(() async {
       when(() => mockStoreRepo.insert<int>(StoreKey.logLevel, any()))
           .thenAnswer((_) async => true);
-      await sut.setlogLevel(LogLevel.shout);
+      await sut.setLogLevel(LogLevel.shout);
     });
 
     test('Updates the log level in store', () {

mobile/test/domain/services/store_service_test.dart

@@ -1,5 +1,3 @@
-// ignore_for_file: avoid-dynamic
-
 import 'dart:async';
 
 import 'package:flutter_test/flutter_test.dart';
@@ -18,10 +16,10 @@ final _kBackupFailedSince = DateTime.utc(2023);
 void main() {
   late StoreService sut;
   late IStoreRepository mockStoreRepo;
-  late StreamController<StoreUpdateEvent> controller;
+  late StreamController<StoreDto<Object>> controller;
 
   setUp(() async {
-    controller = StreamController<StoreUpdateEvent>.broadcast();
+    controller = StreamController<StoreDto<Object>>.broadcast();
     mockStoreRepo = MockStoreRepository();
     // For generics, we need to provide fallback to each concrete type to avoid runtime errors
     registerFallbackValue(StoreKey.accessToken);
@@ -29,18 +27,14 @@ void main() {
     registerFallbackValue(StoreKey.backgroundBackup);
     registerFallbackValue(StoreKey.backupFailedSince);
 
-    when(() => mockStoreRepo.tryGet(any<StoreKey<dynamic>>()))
-        .thenAnswer((invocation) async {
-      final key = invocation.positionalArguments.firstOrNull as StoreKey;
-      return switch (key) {
-        StoreKey.accessToken => _kAccessToken,
-        StoreKey.backgroundBackup => _kBackgroundBackup,
-        StoreKey.groupAssetsBy => _kGroupAssetsBy,
-        StoreKey.backupFailedSince => _kBackupFailedSince,
-        // ignore: avoid-wildcard-cases-with-enums
-        _ => null,
-      };
-    });
+    when(() => mockStoreRepo.getAll()).thenAnswer(
+      (_) async => [
+        const StoreDto(StoreKey.accessToken, _kAccessToken),
+        const StoreDto(StoreKey.backgroundBackup, _kBackgroundBackup),
+        const StoreDto(StoreKey.groupAssetsBy, _kGroupAssetsBy),
+        StoreDto(StoreKey.backupFailedSince, _kBackupFailedSince),
+      ],
+    );
     when(() => mockStoreRepo.watchAll()).thenAnswer((_) => controller.stream);
 
     sut = await StoreService.create(storeRepository: mockStoreRepo);
@@ -53,8 +47,7 @@ void main() {
 
   group("Store Service Init:", () {
     test('Populates the internal cache on init', () {
-      verify(() => mockStoreRepo.tryGet(any<StoreKey<dynamic>>()))
-          .called(equals(StoreKey.values.length));
+      verify(() => mockStoreRepo.getAll()).called(1);
       expect(sut.tryGet(StoreKey.accessToken), _kAccessToken);
       expect(sut.tryGet(StoreKey.backgroundBackup), _kBackgroundBackup);
       expect(sut.tryGet(StoreKey.groupAssetsBy), _kGroupAssetsBy);
@@ -64,8 +57,7 @@ void main() {
     });
 
     test('Listens to stream of store updates', () async {
-      final event =
-          StoreUpdateEvent(StoreKey.accessToken, _kAccessToken.toUpperCase());
+      final event = StoreDto(StoreKey.accessToken, _kAccessToken.toUpperCase());
       controller.add(event);
 
       await pumpEventQueue();

mobile/test/infrastructure/repositories/store_repository_test.dart

@@ -1,5 +1,3 @@
-// ignore_for_file: avoid-dynamic
-
 import 'package:flutter_test/flutter_test.dart';
 import 'package:immich_mobile/domain/interfaces/store.interface.dart';
 import 'package:immich_mobile/domain/models/store.model.dart';
@@ -146,32 +144,21 @@ void main() {
       expectLater(
         stream,
         emitsInAnyOrder([
+          emits(const StoreDto<Object>(StoreKey.version, _kTestVersion)),
           emits(
-            const StoreUpdateEvent<dynamic>(StoreKey.version, _kTestVersion),
+            StoreDto<Object>(StoreKey.backupFailedSince, _kTestBackupFailed),
           ),
           emits(
-            StoreUpdateEvent<dynamic>(
-              StoreKey.backupFailedSince,
-              _kTestBackupFailed,
-            ),
+            const StoreDto<Object>(StoreKey.accessToken, _kTestAccessToken),
           ),
           emits(
-            const StoreUpdateEvent<dynamic>(
-              StoreKey.accessToken,
-              _kTestAccessToken,
-            ),
-          ),
-          emits(
-            const StoreUpdateEvent<dynamic>(
+            const StoreDto<Object>(
               StoreKey.colorfulInterface,
               _kTestColorfulInterface,
             ),
           ),
           emits(
-            const StoreUpdateEvent<dynamic>(
-              StoreKey.version,
-              _kTestVersion + 10,
-            ),
+            const StoreDto<Object>(StoreKey.version, _kTestVersion + 10),
           ),
         ]),
       );