-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat: user sync stream #16862
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
feat: user sync stream #16862
Changes from 23 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
49eb5ee
refactor: user entity
shenlong-tanwen fea9a16
chore: rebase fixes
shenlong-tanwen 155a2e8
refactor: remove int user Id
shenlong-tanwen 3f7fa90
refactor: migrate store userId from int to string
shenlong-tanwen c9309d6
refactor: rename uid to id
shenlong-tanwen 5fa42c7
feat: drift
shenlong-tanwen aa32bed
pr feedback
shenlong-tanwen df65f70
refactor: move common overrides to mixin
shenlong-tanwen 24d9dac
refactor: remove int user Id
shenlong-tanwen c964067
refactor: migrate store userId from int to string
shenlong-tanwen 734566d
refactor: rename uid to id
shenlong-tanwen 3739e86
feat: user & partner sync stream
shenlong-tanwen 7246bcf
pr changes
shenlong-tanwen 6baff23
refactor: sync service and add tests
shenlong-tanwen bc53f5c
chore: remove generated change
shenlong-tanwen 3dc2540
chore: move sync model
shenlong-tanwen 1fdb78b
rebase: convert string ids to byte uuids
shenlong-tanwen afd664d
rebase
shenlong-tanwen 84d665e
Merge branch 'main' of github.com:immich-app/immich into feat/user-sy…
alextran1502 70263c5
add processing logs
shenlong-tanwen 52cca7a
batch db calls
shenlong-tanwen deca45a
rewrite isolate manager
shenlong-tanwen 92210b3
Merge branch 'main' into feat/user-sync-stream
alextran1502 c427f52
rewrite with worker_manager
shenlong-tanwen b72a91a
Merge branch 'main' of github.com:immich-app/immich into feat/user-sy…
alextran1502 04e6e56
misc fixes
shenlong-tanwen 13ef491
add sync order test
shenlong-tanwen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
description: This file stores settings for Dart & Flutter DevTools. | ||
documentation: https://docs.flutter.dev/tools/devtools/extensions#configure-extension-enablement-states | ||
extensions: | ||
- drift: true |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
import 'package:immich_mobile/domain/models/sync/sync_event.model.dart'; | ||
import 'package:immich_mobile/domain/models/sync_event.model.dart'; | ||
import 'package:openapi/api.dart'; | ||
|
||
abstract interface class ISyncApiRepository { | ||
Future<void> ack(String data); | ||
Future<void> ack(List<String> data); | ||
|
||
Stream<List<SyncEvent>> watchUserSyncEvent(); | ||
Stream<List<SyncEvent>> getSyncEvents(List<SyncRequestType> type); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import 'package:immich_mobile/domain/interfaces/db.interface.dart'; | ||
import 'package:openapi/api.dart'; | ||
|
||
abstract interface class ISyncStreamRepository implements IDatabaseRepository { | ||
Future<bool> updateUsersV1(Iterable<SyncUserV1> data); | ||
Future<bool> deleteUsersV1(Iterable<SyncUserDeleteV1> data); | ||
|
||
Future<bool> updatePartnerV1(Iterable<SyncPartnerV1> data); | ||
Future<bool> deletePartnerV1(Iterable<SyncPartnerDeleteV1> data); | ||
} |
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import 'package:openapi/api.dart'; | ||
|
||
class SyncEvent { | ||
final SyncEntityType type; | ||
// ignore: avoid-dynamic | ||
final dynamic data; | ||
final String ack; | ||
|
||
const SyncEvent({required this.type, required this.data, required this.ack}); | ||
|
||
@override | ||
String toString() => 'SyncEvent(type: $type, data: $data, ack: $ack)'; | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,49 +1,175 @@ | ||
// ignore_for_file: avoid-passing-async-when-sync-expected | ||
|
||
import 'dart:async'; | ||
|
||
import 'package:flutter/foundation.dart'; | ||
import 'package:collection/collection.dart'; | ||
import 'package:immich_mobile/domain/interfaces/sync_api.interface.dart'; | ||
import 'package:immich_mobile/domain/interfaces/sync_stream.interface.dart'; | ||
import 'package:immich_mobile/domain/utils/cancel.exception.dart'; | ||
import 'package:logging/logging.dart'; | ||
import 'package:openapi/api.dart'; | ||
|
||
class SyncStreamService { | ||
final Logger _logger = Logger('SyncStreamService'); | ||
|
||
final ISyncApiRepository _syncApiRepository; | ||
final ISyncStreamRepository _syncStreamRepository; | ||
final Completer<bool>? _cancelCompleter; | ||
|
||
SyncStreamService({ | ||
required ISyncApiRepository syncApiRepository, | ||
required ISyncStreamRepository syncStreamRepository, | ||
Completer<bool>? cancelCompleter, | ||
}) : _syncApiRepository = syncApiRepository, | ||
_syncStreamRepository = syncStreamRepository, | ||
_cancelCompleter = cancelCompleter; | ||
|
||
SyncStreamService(this._syncApiRepository); | ||
Future<bool> _handleSyncData( | ||
SyncEntityType type, | ||
// ignore: avoid-dynamic | ||
Iterable<dynamic> data, | ||
) async { | ||
if (data.isEmpty) { | ||
_logger.warning("Received empty sync data for $type"); | ||
return false; | ||
} | ||
|
||
StreamSubscription? _userSyncSubscription; | ||
_logger.fine("Processing sync data for $type of length ${data.length}"); | ||
|
||
void syncUsers() { | ||
_userSyncSubscription = | ||
_syncApiRepository.watchUserSyncEvent().listen((events) async { | ||
for (final event in events) { | ||
if (event.data is SyncUserV1) { | ||
final data = event.data as SyncUserV1; | ||
debugPrint("User Update: $data"); | ||
try { | ||
if (type == SyncEntityType.partnerV1) { | ||
return await _syncStreamRepository.updatePartnerV1(data.cast()); | ||
shenlong-tanwen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
if (type == SyncEntityType.partnerDeleteV1) { | ||
return await _syncStreamRepository.deletePartnerV1(data.cast()); | ||
} | ||
|
||
// final user = await _userRepository.get(data.id); | ||
if (type == SyncEntityType.userV1) { | ||
return await _syncStreamRepository.updateUsersV1(data.cast()); | ||
} | ||
|
||
// if (user == null) { | ||
// continue; | ||
// } | ||
if (type == SyncEntityType.userDeleteV1) { | ||
return await _syncStreamRepository.deleteUsersV1(data.cast()); | ||
} | ||
} catch (error, stack) { | ||
_logger.severe("Error processing sync data for $type", error, stack); | ||
return false; | ||
} | ||
|
||
// user.name = data.name; | ||
// user.email = data.email; | ||
// user.updatedAt = DateTime.now(); | ||
_logger.warning("Unknown sync data type: $type"); | ||
return false; | ||
} | ||
|
||
// await _userRepository.update(user); | ||
// await _syncApiRepository.ack(event.ack); | ||
Future<void> _syncEvent(List<SyncRequestType> types) async { | ||
_logger.info("Syncing Events: $types"); | ||
final streamCompleter = Completer(); | ||
bool shouldComplete = false; | ||
// the onDone callback might fire before the events are processed | ||
// the following flag ensures that the onDone callback is not called | ||
// before the events are processed and also that events are processed sequentially | ||
Completer? mutex; | ||
StreamSubscription? subscription; | ||
subscription = _syncApiRepository.getSyncEvents(types).listen( | ||
(events) async { | ||
if (events.isEmpty) { | ||
_logger.warning("Received empty sync events"); | ||
return; | ||
} | ||
|
||
if (event.data is SyncUserDeleteV1) { | ||
final data = event.data as SyncUserDeleteV1; | ||
// If previous events are still being processed, wait for them to finish | ||
if (mutex != null) { | ||
await mutex!.future; | ||
} | ||
|
||
debugPrint("User delete: $data"); | ||
// await _syncApiRepository.ack(event.ack); | ||
if (_cancelCompleter?.isCompleted ?? false) { | ||
_logger.info("Sync cancelled, stopping stream"); | ||
subscription?.cancel(); | ||
if (!streamCompleter.isCompleted) { | ||
streamCompleter.completeError( | ||
const CancelException(), | ||
StackTrace.current, | ||
); | ||
} | ||
return; | ||
} | ||
} | ||
|
||
// Take control of the mutex and process the events | ||
mutex = Completer(); | ||
|
||
try { | ||
final eventsMap = events.groupListsBy((event) => event.type); | ||
final Map<SyncEntityType, String> acks = {}; | ||
|
||
for (final entry in eventsMap.entries) { | ||
if (_cancelCompleter?.isCompleted ?? false) { | ||
_logger.info("Sync cancelled, stopping stream"); | ||
mutex?.complete(); | ||
mutex = null; | ||
if (!streamCompleter.isCompleted) { | ||
streamCompleter.completeError( | ||
const CancelException(), | ||
StackTrace.current, | ||
); | ||
} | ||
|
||
return; | ||
} | ||
|
||
final type = entry.key; | ||
final data = entry.value; | ||
|
||
if (data.isEmpty) { | ||
_logger.warning("Received empty sync events for $type"); | ||
continue; | ||
} | ||
|
||
if (await _handleSyncData(type, data.map((e) => e.data))) { | ||
// ignore: avoid-unsafe-collection-methods | ||
acks[type] = data.last.ack; | ||
} else { | ||
_logger.warning("Failed to handle sync events for $type"); | ||
} | ||
} | ||
|
||
if (acks.isNotEmpty) { | ||
await _syncApiRepository.ack(acks.values.toList()); | ||
} | ||
_logger.info("$types events processed"); | ||
} catch (error, stack) { | ||
_logger.warning("Error handling sync events", error, stack); | ||
} finally { | ||
mutex?.complete(); | ||
mutex = null; | ||
} | ||
|
||
if (shouldComplete) { | ||
_logger.info("Sync done, completing stream"); | ||
if (!streamCompleter.isCompleted) streamCompleter.complete(); | ||
} | ||
}, | ||
onError: (error, stack) { | ||
_logger.warning("Error in sync stream for $types", error, stack); | ||
// Do not proceed if the stream errors | ||
if (!streamCompleter.isCompleted) streamCompleter.complete(); | ||
}, | ||
onDone: () { | ||
_logger.info("$types stream done"); | ||
if (mutex == null && !streamCompleter.isCompleted) { | ||
streamCompleter.complete(); | ||
} else { | ||
// Marks the stream as done but does not complete the completer | ||
// until the events are processed | ||
shouldComplete = true; | ||
} | ||
}, | ||
); | ||
return await streamCompleter.future.whenComplete(() { | ||
_logger.info("Sync stream completed"); | ||
return subscription?.cancel(); | ||
}); | ||
} | ||
|
||
Future<void> dispose() async { | ||
await _userSyncSubscription?.cancel(); | ||
} | ||
Future<void> syncUsers() => | ||
_syncEvent([SyncRequestType.usersV1, SyncRequestType.partnersV1]); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
// ignore_for_file: avoid-passing-async-when-sync-expected | ||
|
||
import 'dart:async'; | ||
|
||
import 'package:async/async.dart'; | ||
import 'package:immich_mobile/providers/infrastructure/cancel.provider.dart'; | ||
import 'package:immich_mobile/providers/infrastructure/sync_stream.provider.dart'; | ||
import 'package:immich_mobile/utils/isolate.dart'; | ||
|
||
class BackgroundSyncManager { | ||
CancelableOperation<void>? _userSyncFuture; | ||
|
||
BackgroundSyncManager(); | ||
|
||
Future<void> cancel() async { | ||
await _userSyncFuture?.cancel(); | ||
_userSyncFuture = null; | ||
} | ||
|
||
Future<void> syncUsers() async { | ||
shenlong-tanwen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (_userSyncFuture != null) { | ||
return _userSyncFuture!.valueOrCancellation(); | ||
} | ||
|
||
if (_userSyncFuture == null) { | ||
final isolate = await IsolateManager.spawn( | ||
computation: (ref) => ref.read(syncStreamServiceProvider).syncUsers(), | ||
alextran1502 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
onCancel: (ref) => ref.read(cancellationProvider).complete(true), | ||
); | ||
_userSyncFuture = CancelableOperation.fromFuture( | ||
isolate.run(), | ||
onCancel: () { | ||
isolate.cancel(); | ||
_userSyncFuture = null; | ||
}, | ||
); | ||
return _userSyncFuture! | ||
.valueOrCancellation() | ||
.then((_) => _userSyncFuture = null); | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
class CancelException implements Exception { | ||
final String message; | ||
|
||
const CancelException([this.message = "Operation was cancelled."]); | ||
|
||
@override | ||
String toString() => "CancelException: $message"; | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should avoid dynamic to the extent possible. Ideally, only the JSON parsing would involve dynamic, and parse directly into strongly typed objects that share a type (either through an interface, by inheritance or maybe through generics). As it is, dynamic leaks into the events, the response converter map, the main sync handler, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, we went with having strongly typed DTOs initially. But it resulted in us creating a lot of duplicate domain classes that were already available from the OpenAPI generated code. The generated codes are not strongly typed so we do not have a common base class that we could use here.