diff --git a/mobile/lib/domain/interfaces/store.interface.dart b/mobile/lib/domain/interfaces/store.interface.dart index 7a45f9dbe0..b0a6762566 100644 --- a/mobile/lib/domain/interfaces/store.interface.dart +++ b/mobile/lib/domain/interfaces/store.interface.dart @@ -6,9 +6,11 @@ abstract interface class IStoreRepository implements IDatabaseRepository { Future tryGet(StoreKey key); + Future>> getAll(); + Stream watch(StoreKey key); - Stream watchAll(); + Stream> watchAll(); Future update(StoreKey key, T value); diff --git a/mobile/lib/domain/models/store.model.dart b/mobile/lib/domain/models/store.model.dart index 8a5a908e0d..a96e8d3bce 100644 --- a/mobile/lib/domain/models/store.model.dart +++ b/mobile/lib/domain/models/store.model.dart @@ -75,23 +75,23 @@ enum StoreKey { Type get type => T; } -class StoreUpdateEvent { +class StoreDto { final StoreKey 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 ?? ''}, }'''; } @override - bool operator ==(covariant StoreUpdateEvent other) { + bool operator ==(covariant StoreDto other) { if (identical(this, other)) return true; return other.key == key && other.value == value; diff --git a/mobile/lib/domain/models/sync_event.model.dart b/mobile/lib/domain/models/sync_event.model.dart index 2ad8a75fe1..d78769ceac 100644 --- a/mobile/lib/domain/models/sync_event.model.dart +++ b/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}); diff --git a/mobile/lib/domain/services/log.service.dart b/mobile/lib/domain/services/log.service.dart index 2136912a67..a25147d185 100644 --- a/mobile/lib/domain/services/log.service.dart +++ b/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 _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 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> getMessages() async { - final logsFromDb = await _logRepository.getAll(); - if (_msgBuffer.isNotEmpty) { - return [..._msgBuffer.reversed, ...logsFromDb]; - } - return logsFromDb; - } - - Future 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 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 _flushBufferToDatabase() async { + Future setLogLevel(LogLevel level) async { + await _storeRepository.insert(StoreKey.logLevel, level.index); + Logger.root.level = level.toLevel(); + } + + Future> getMessages() async { + final logsFromDb = await _logRepository.getAll(); + return [..._msgBuffer.reversed, ...logsFromDb]; + } + + Future 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 dispose() { + _flushTimer?.cancel(); + _logSubscription.cancel(); + return flushBuffer(); + } + + // TOOD: Move this to private once Isar is removed + Future flushBuffer() async { _flushTimer = null; final buffer = [..._msgBuffer]; _msgBuffer.clear(); diff --git a/mobile/lib/domain/services/store.service.dart b/mobile/lib/domain/services/store.service.dart index 73426cbf4e..7b71acd254 100644 --- a/mobile/lib/domain/services/store.service.dart +++ b/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 _cache = {}; - late final StreamSubscription _storeUpdateSubscription; + /// In-memory cache. Keys are [StoreKey.id] + final Map _cache = {}; + late final StreamSubscription _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 init({ required IStoreRepository storeRepository, }) async { @@ -31,7 +32,6 @@ class StoreService { return _instance!; } - /// Initializes the store with the given [storeRepository] static Future create({ required IStoreRepository storeRepository, }) async { @@ -41,16 +41,14 @@ class StoreService { return instance; } - /// Fills the cache with the values from the DB Future _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 _listenForChange() => + StreamSubscription _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(StoreKey key) => _cache[key.id]; + /// Returns the cached value for [key], or `null` + T? tryGet(StoreKey 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(StoreKey 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 put, 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 watch(StoreKey key) => _storeRepository.watch(key); - /// Removes the value asynchronously from the Store + /// Removes the value for [key] Future delete(StoreKey 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 clear() async { await _storeRepository.deleteAll(); _cache.clear(); diff --git a/mobile/lib/domain/services/sync_stream.service.dart b/mobile/lib/domain/services/sync_stream.service.dart index ac63734b07..f0b0887c4c 100644 --- a/mobile/lib/domain/services/sync_stream.service.dart +++ b/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 _handleSyncData( SyncEntityType type, - // ignore: avoid-dynamic - Iterable data, + Iterable data, ) async { _logger.fine("Processing sync data for $type of length ${data.length}"); // ignore: prefer-switch-expression diff --git a/mobile/lib/domain/utils/background_sync.dart b/mobile/lib/domain/utils/background_sync.dart index f63dc81ba9..ca4efc938d 100644 --- a/mobile/lib/domain/utils/background_sync.dart +++ b/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; } } diff --git a/mobile/lib/infrastructure/repositories/store.repository.dart b/mobile/lib/infrastructure/repositories/store.repository.dart index e8769c5084..fec36193bc 100644 --- a/mobile/lib/infrastructure/repositories/store.repository.dart +++ b/mobile/lib/infrastructure/repositories/store.repository.dart @@ -22,7 +22,7 @@ class IsarStoreRepository extends IsarDatabaseRepository } @override - Stream watchAll() { + Stream> 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 _toUpdateEvent(StoreValue entity) async { - final key = StoreKey.values.firstWhere((e) => e.id == entity.id); + Future> _toUpdateEvent(StoreValue entity) async { + final key = StoreKey.values.firstWhere((e) => e.id == entity.id) + as StoreKey; final value = await _toValue(key, entity); - return StoreUpdateEvent(key, value); + return StoreDto(key, value); } Future _toValue(StoreKey key, StoreValue entity) async => @@ -107,4 +108,13 @@ class IsarStoreRepository extends IsarDatabaseRepository }; return StoreValue(key.id, intValue: intValue, strValue: strValue); } + + @override + Future>> 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()); + } } diff --git a/mobile/lib/infrastructure/repositories/sync_api.repository.dart b/mobile/lib/infrastructure/repositories/sync_api.repository.dart index dd1ea208ba..f71e4403e6 100644 --- a/mobile/lib/infrastructure/repositories/sync_api.repository.dart +++ b/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 = { +const _kResponseMap = { SyncEntityType.userV1: SyncUserV1.fromJson, SyncEntityType.userDeleteV1: SyncUserDeleteV1.fromJson, SyncEntityType.partnerV1: SyncPartnerV1.fromJson, diff --git a/mobile/lib/utils/isolate.dart b/mobile/lib/utils/isolate.dart index cfbb1b544f..6b20fa7f37 100644 --- a/mobile/lib/utils/isolate.dart +++ b/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 runInIsolateGentle({ 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(); } diff --git a/mobile/lib/widgets/settings/advanced_settings.dart b/mobile/lib/widgets/settings/advanced_settings.dart index 98c8728298..ff7afc1748 100644 --- a/mobile/lib/widgets/settings/advanced_settings.dart +++ b/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 checkAndroidVersion() async { diff --git a/mobile/test/domain/services/log_service_test.dart b/mobile/test/domain/services/log_service_test.dart index 5811a8c430..fd9a86ce4e 100644 --- a/mobile/test/domain/services/log_service_test.dart +++ b/mobile/test/domain/services/log_service_test.dart @@ -74,7 +74,7 @@ void main() { setUp(() async { when(() => mockStoreRepo.insert(StoreKey.logLevel, any())) .thenAnswer((_) async => true); - await sut.setlogLevel(LogLevel.shout); + await sut.setLogLevel(LogLevel.shout); }); test('Updates the log level in store', () { diff --git a/mobile/test/domain/services/store_service_test.dart b/mobile/test/domain/services/store_service_test.dart index 554ca73500..8f4749ac62 100644 --- a/mobile/test/domain/services/store_service_test.dart +++ b/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 controller; + late StreamController> controller; setUp(() async { - controller = StreamController.broadcast(); + controller = StreamController>.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>())) - .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>())) - .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(); diff --git a/mobile/test/infrastructure/repositories/store_repository_test.dart b/mobile/test/infrastructure/repositories/store_repository_test.dart index 16e0632d83..528e17ba3d 100644 --- a/mobile/test/infrastructure/repositories/store_repository_test.dart +++ b/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(StoreKey.version, _kTestVersion)), emits( - const StoreUpdateEvent(StoreKey.version, _kTestVersion), + StoreDto(StoreKey.backupFailedSince, _kTestBackupFailed), ), emits( - StoreUpdateEvent( - StoreKey.backupFailedSince, - _kTestBackupFailed, - ), + const StoreDto(StoreKey.accessToken, _kTestAccessToken), ), emits( - const StoreUpdateEvent( - StoreKey.accessToken, - _kTestAccessToken, - ), - ), - emits( - const StoreUpdateEvent( + const StoreDto( StoreKey.colorfulInterface, _kTestColorfulInterface, ), ), emits( - const StoreUpdateEvent( - StoreKey.version, - _kTestVersion + 10, - ), + const StoreDto(StoreKey.version, _kTestVersion + 10), ), ]), );