From ed5e21cce81c4ac83f88018aa630d21b264ad3b0 Mon Sep 17 00:00:00 2001 From: murali-shris Date: Wed, 25 Oct 2023 17:25:10 +0530 Subject: [PATCH 1/4] fix: pass enrollmentId to notification and sync service through default service factory --- .../lib/src/manager/at_client_manager.dart | 34 +++++++++++-------- .../at_client/lib/src/manager/monitor.dart | 1 + .../service/notification_service_impl.dart | 16 ++++++--- .../lib/src/service/sync_service_impl.dart | 5 +-- .../test/test_utils/no_op_services.dart | 10 +++--- 5 files changed, 40 insertions(+), 26 deletions(-) diff --git a/packages/at_client/lib/src/manager/at_client_manager.dart b/packages/at_client/lib/src/manager/at_client_manager.dart index 0880ce091..ba5639843 100644 --- a/packages/at_client/lib/src/manager/at_client_manager.dart +++ b/packages/at_client/lib/src/manager/at_client_manager.dart @@ -79,19 +79,22 @@ class AtClientManager { 'Switching atSigns from ${_currentAtClient?.getCurrentAtSign()} to $atSign'); _atSign = atSign; var previousAtClient = _currentAtClient; - _currentAtClient = await serviceFactory - .atClient(_atSign, namespace, preference, this, atChops: atChops); + _currentAtClient = await serviceFactory.atClient( + _atSign, namespace, preference, this, + atChops: atChops, enrollmentId: enrollmentId); final switchAtSignEvent = SwitchAtSignEvent(previousAtClient, _currentAtClient!); _notifyListeners(switchAtSignEvent); - var notificationService = - await serviceFactory.notificationService(_currentAtClient!, this); + var notificationService = await serviceFactory.notificationService( + _currentAtClient!, this, + enrollmentId: enrollmentId); _currentAtClient!.notificationService = notificationService; var syncService = await serviceFactory.syncService( - _currentAtClient!, this, notificationService); + _currentAtClient!, this, notificationService, + enrollmentId: enrollmentId); _currentAtClient!.syncService = syncService; _logger.info("setCurrentAtSign complete"); @@ -147,10 +150,12 @@ abstract class AtServiceFactory { {AtChops? atChops, String? enrollmentId}); Future notificationService( - AtClient atClient, AtClientManager atClientManager); + AtClient atClient, AtClientManager atClientManager, + {String? enrollmentId}); Future syncService(AtClient atClient, - AtClientManager atClientManager, NotificationService notificationService); + AtClientManager atClientManager, NotificationService notificationService, + {String? enrollmentId}); } class DefaultAtServiceFactory implements AtServiceFactory { @@ -166,18 +171,19 @@ class DefaultAtServiceFactory implements AtServiceFactory { @override Future notificationService( - AtClient atClient, AtClientManager atClientManager) async { + AtClient atClient, AtClientManager atClientManager, + {String? enrollmentId}) async { return await NotificationServiceImpl.create(atClient, - atClientManager: atClientManager); + atClientManager: atClientManager, enrollmentId: enrollmentId); } @override - Future syncService( - AtClient atClient, - AtClientManager atClientManager, - NotificationService notificationService) async { + Future syncService(AtClient atClient, + AtClientManager atClientManager, NotificationService notificationService, + {String? enrollmentId}) async { return await SyncServiceImpl.create(atClient, atClientManager: atClientManager, - notificationService: notificationService); + notificationService: notificationService, + enrollmentId: enrollmentId); } } diff --git a/packages/at_client/lib/src/manager/monitor.dart b/packages/at_client/lib/src/manager/monitor.dart index 528bf41c2..48c364f18 100644 --- a/packages/at_client/lib/src/manager/monitor.dart +++ b/packages/at_client/lib/src/manager/monitor.dart @@ -138,6 +138,7 @@ class Monitor { _keepAlive = monitorPreference.keepAlive; _lastNotificationTime = monitorPreference.lastNotificationTime; _enrollmentId = enrollmentId; + _logger.finer('enrollmentId: $_enrollmentId'); _remoteSecondary = remoteSecondary ?? RemoteSecondary(atSign, preference, atChops: atChops, enrollmentId: enrollmentId); diff --git a/packages/at_client/lib/src/service/notification_service_impl.dart b/packages/at_client/lib/src/service/notification_service_impl.dart index e19844fb4..d27247cad 100644 --- a/packages/at_client/lib/src/service/notification_service_impl.dart +++ b/packages/at_client/lib/src/service/notification_service_impl.dart @@ -76,20 +76,25 @@ class NotificationServiceImpl String get currentAtSign => _atClient.getCurrentAtSign()!; static Future create(AtClient atClient, - {required AtClientManager atClientManager, Monitor? monitor}) async { - final notificationService = - NotificationServiceImpl._(atClientManager, atClient, monitor: monitor); + {required AtClientManager atClientManager, + Monitor? monitor, + String? enrollmentId}) async { + final notificationService = NotificationServiceImpl._( + atClientManager, atClient, + monitor: monitor, enrollmentId: enrollmentId); // We used to call _init() at this point which would start the monitor, but now we // call _init() from the [subscribe] method return notificationService; } NotificationServiceImpl._(AtClientManager atClientManager, AtClient atClient, - {Monitor? monitor}) { + {Monitor? monitor, String? enrollmentId}) { _atClientManager = atClientManager; _atClient = atClient; _logger = AtSignLogger( 'NotificationServiceImpl (${_atClient.getCurrentAtSign()})'); + + _logger.info('enrollmentId: $enrollmentId'); _monitor = monitor ?? Monitor( _internalNotificationCallback, @@ -98,7 +103,8 @@ class NotificationServiceImpl _atClient.getPreferences()!, MonitorPreference()..keepAlive = true, monitorRetry, - atChops: atClient.atChops); + atChops: atClient.atChops, + enrollmentId: enrollmentId); _atClientManager.listenToAtSignChange(this); lastReceivedNotificationAtKey = AtKey.local( lastReceivedNotificationKey, _atClient.getCurrentAtSign()!, diff --git a/packages/at_client/lib/src/service/sync_service_impl.dart b/packages/at_client/lib/src/service/sync_service_impl.dart index 92069d443..3a298910d 100644 --- a/packages/at_client/lib/src/service/sync_service_impl.dart +++ b/packages/at_client/lib/src/service/sync_service_impl.dart @@ -69,10 +69,11 @@ class SyncServiceImpl implements SyncService, AtSignChangeListener { static Future create(AtClient atClient, {required AtClientManager atClientManager, required NotificationService notificationService, - RemoteSecondary? remoteSecondary}) async { + RemoteSecondary? remoteSecondary, + String? enrollmentId}) async { remoteSecondary ??= RemoteSecondary( atClient.getCurrentAtSign()!, atClient.getPreferences()!, - atChops: atClient.atChops); + atChops: atClient.atChops, enrollmentId: enrollmentId); final syncService = SyncServiceImpl._( atClientManager, atClient, notificationService, remoteSecondary); await syncService.statsServiceListener(); diff --git a/packages/at_client/test/test_utils/no_op_services.dart b/packages/at_client/test/test_utils/no_op_services.dart index e686f8a1e..e16ccc1c6 100644 --- a/packages/at_client/test/test_utils/no_op_services.dart +++ b/packages/at_client/test/test_utils/no_op_services.dart @@ -2,16 +2,16 @@ import 'package:at_client/at_client.dart'; class ServiceFactoryWithNoOpServices extends DefaultAtServiceFactory { @override - Future syncService( - AtClient atClient, - AtClientManager atClientManager, - NotificationService notificationService) async { + Future syncService(AtClient atClient, + AtClientManager atClientManager, NotificationService notificationService, + {String? enrollmentId}) async { return NoOpSyncService(); } @override Future notificationService( - AtClient atClient, AtClientManager atClientManager) async { + AtClient atClient, AtClientManager atClientManager, + {String? enrollmentId}) async { return NoOpNotificationService(); } } From 3c1cd37b78d8e49b1eed721e4814828d1e1bb30b Mon Sep 17 00:00:00 2001 From: murali-shris Date: Thu, 26 Oct 2023 13:00:22 +0530 Subject: [PATCH 2/4] fix: use enrollmentId from atClient in sync/notification service --- .../lib/src/client/at_client_impl.dart | 10 ++--- .../lib/src/client/at_client_spec.dart | 5 +++ .../lib/src/manager/at_client_manager.dart | 44 ++++++++----------- .../service/notification_service_impl.dart | 15 +++---- .../lib/src/service/sync_service_impl.dart | 5 +-- .../test/test_utils/no_op_services.dart | 10 ++--- 6 files changed, 40 insertions(+), 49 deletions(-) diff --git a/packages/at_client/lib/src/client/at_client_impl.dart b/packages/at_client/lib/src/client/at_client_impl.dart index ef7ab8801..1bdae43dc 100644 --- a/packages/at_client/lib/src/client/at_client_impl.dart +++ b/packages/at_client/lib/src/client/at_client_impl.dart @@ -114,8 +114,6 @@ class AtClientImpl implements AtClient, AtSignChangeListener { late final AtSignLogger _logger; - String? _enrollmentId; - @visibleForTesting static final Map atClientInstanceMap = {}; @@ -126,7 +124,6 @@ class AtClientImpl implements AtClient, AtSignChangeListener { EncryptionService? encryptionService, SecondaryKeyStore? localSecondaryKeyStore, AtChops? atChops, - String? enrollmentId, AtClientCommitLogCompaction? atClientCommitLogCompaction, AtClientConfig? atClientConfig}) async { atClientManager ??= AtClientManager.getInstance(); @@ -162,7 +159,6 @@ class AtClientImpl implements AtClient, AtSignChangeListener { EncryptionService? encryptionService, SecondaryKeyStore? localSecondaryKeyStore, AtChops? atChops, - String? enrollmentId, AtClientCommitLogCompaction? atClientCommitLogCompaction, AtClientConfig? atClientConfig}) { _atSign = AtUtils.fixAtSign(theAtSign); @@ -179,7 +175,6 @@ class AtClientImpl implements AtClient, AtSignChangeListener { _remoteSecondary = remoteSecondary; _encryptionService = encryptionService; _atChops = atChops; - _enrollmentId = enrollmentId; _atClientCommitLogCompaction = atClientCommitLogCompaction; } @@ -198,7 +193,7 @@ class AtClientImpl implements AtClient, AtSignChangeListener { _remoteSecondary ??= RemoteSecondary(_atSign, _preference!, atChops: atChops, privateKey: _preference!.privateKey, - enrollmentId: _enrollmentId); + enrollmentId: enrollmentId); // Now using ??= because we may be injecting an EncryptionService _encryptionService ??= EncryptionService(_atSign); @@ -996,4 +991,7 @@ class AtClientImpl implements AtClient, AtSignChangeListener { } return result.notificationID; } + + @override + String? enrollmentId; } diff --git a/packages/at_client/lib/src/client/at_client_spec.dart b/packages/at_client/lib/src/client/at_client_spec.dart index 06af3a85f..d04ec9424 100644 --- a/packages/at_client/lib/src/client/at_client_spec.dart +++ b/packages/at_client/lib/src/client/at_client_spec.dart @@ -38,6 +38,11 @@ abstract class AtClient { AtChops? get atChops; + /// Enrollment id for apkam enrolled clients + set enrollmentId(String? enrollmentId); + + String? get enrollmentId; + set syncService(SyncService syncService); SyncService get syncService; diff --git a/packages/at_client/lib/src/manager/at_client_manager.dart b/packages/at_client/lib/src/manager/at_client_manager.dart index ba5639843..68e3537e2 100644 --- a/packages/at_client/lib/src/manager/at_client_manager.dart +++ b/packages/at_client/lib/src/manager/at_client_manager.dart @@ -79,22 +79,19 @@ class AtClientManager { 'Switching atSigns from ${_currentAtClient?.getCurrentAtSign()} to $atSign'); _atSign = atSign; var previousAtClient = _currentAtClient; - _currentAtClient = await serviceFactory.atClient( - _atSign, namespace, preference, this, - atChops: atChops, enrollmentId: enrollmentId); - + _currentAtClient = await serviceFactory + .atClient(_atSign, namespace, preference, this, atChops: atChops); + _currentAtClient?.enrollmentId = enrollmentId; final switchAtSignEvent = SwitchAtSignEvent(previousAtClient, _currentAtClient!); _notifyListeners(switchAtSignEvent); - var notificationService = await serviceFactory.notificationService( - _currentAtClient!, this, - enrollmentId: enrollmentId); + var notificationService = + await serviceFactory.notificationService(_currentAtClient!, this); _currentAtClient!.notificationService = notificationService; var syncService = await serviceFactory.syncService( - _currentAtClient!, this, notificationService, - enrollmentId: enrollmentId); + _currentAtClient!, this, notificationService); _currentAtClient!.syncService = syncService; _logger.info("setCurrentAtSign complete"); @@ -147,43 +144,38 @@ class AtClientManager { abstract class AtServiceFactory { Future atClient(String atSign, String? namespace, AtClientPreference preference, AtClientManager atClientManager, - {AtChops? atChops, String? enrollmentId}); + {AtChops? atChops}); Future notificationService( - AtClient atClient, AtClientManager atClientManager, - {String? enrollmentId}); + AtClient atClient, AtClientManager atClientManager); Future syncService(AtClient atClient, - AtClientManager atClientManager, NotificationService notificationService, - {String? enrollmentId}); + AtClientManager atClientManager, NotificationService notificationService); } class DefaultAtServiceFactory implements AtServiceFactory { @override Future atClient(String atSign, String? namespace, AtClientPreference preference, AtClientManager atClientManager, - {AtChops? atChops, String? enrollmentId}) async { + {AtChops? atChops}) async { return await AtClientImpl.create(atSign, namespace, preference, - atClientManager: atClientManager, - atChops: atChops, - enrollmentId: enrollmentId); + atClientManager: atClientManager, atChops: atChops); } @override Future notificationService( - AtClient atClient, AtClientManager atClientManager, - {String? enrollmentId}) async { + AtClient atClient, AtClientManager atClientManager) async { return await NotificationServiceImpl.create(atClient, - atClientManager: atClientManager, enrollmentId: enrollmentId); + atClientManager: atClientManager); } @override - Future syncService(AtClient atClient, - AtClientManager atClientManager, NotificationService notificationService, - {String? enrollmentId}) async { + Future syncService( + AtClient atClient, + AtClientManager atClientManager, + NotificationService notificationService) async { return await SyncServiceImpl.create(atClient, atClientManager: atClientManager, - notificationService: notificationService, - enrollmentId: enrollmentId); + notificationService: notificationService); } } diff --git a/packages/at_client/lib/src/service/notification_service_impl.dart b/packages/at_client/lib/src/service/notification_service_impl.dart index d27247cad..1e9c5ed84 100644 --- a/packages/at_client/lib/src/service/notification_service_impl.dart +++ b/packages/at_client/lib/src/service/notification_service_impl.dart @@ -76,25 +76,22 @@ class NotificationServiceImpl String get currentAtSign => _atClient.getCurrentAtSign()!; static Future create(AtClient atClient, - {required AtClientManager atClientManager, - Monitor? monitor, - String? enrollmentId}) async { - final notificationService = NotificationServiceImpl._( - atClientManager, atClient, - monitor: monitor, enrollmentId: enrollmentId); + {required AtClientManager atClientManager, Monitor? monitor}) async { + final notificationService = + NotificationServiceImpl._(atClientManager, atClient, monitor: monitor); // We used to call _init() at this point which would start the monitor, but now we // call _init() from the [subscribe] method return notificationService; } NotificationServiceImpl._(AtClientManager atClientManager, AtClient atClient, - {Monitor? monitor, String? enrollmentId}) { + {Monitor? monitor}) { _atClientManager = atClientManager; _atClient = atClient; _logger = AtSignLogger( 'NotificationServiceImpl (${_atClient.getCurrentAtSign()})'); - _logger.info('enrollmentId: $enrollmentId'); + _logger.finer('enrollmentId: ${atClient.enrollmentId}'); _monitor = monitor ?? Monitor( _internalNotificationCallback, @@ -104,7 +101,7 @@ class NotificationServiceImpl MonitorPreference()..keepAlive = true, monitorRetry, atChops: atClient.atChops, - enrollmentId: enrollmentId); + enrollmentId: atClient.enrollmentId); _atClientManager.listenToAtSignChange(this); lastReceivedNotificationAtKey = AtKey.local( lastReceivedNotificationKey, _atClient.getCurrentAtSign()!, diff --git a/packages/at_client/lib/src/service/sync_service_impl.dart b/packages/at_client/lib/src/service/sync_service_impl.dart index 3a298910d..8eefc565f 100644 --- a/packages/at_client/lib/src/service/sync_service_impl.dart +++ b/packages/at_client/lib/src/service/sync_service_impl.dart @@ -69,11 +69,10 @@ class SyncServiceImpl implements SyncService, AtSignChangeListener { static Future create(AtClient atClient, {required AtClientManager atClientManager, required NotificationService notificationService, - RemoteSecondary? remoteSecondary, - String? enrollmentId}) async { + RemoteSecondary? remoteSecondary}) async { remoteSecondary ??= RemoteSecondary( atClient.getCurrentAtSign()!, atClient.getPreferences()!, - atChops: atClient.atChops, enrollmentId: enrollmentId); + atChops: atClient.atChops, enrollmentId: atClient.enrollmentId); final syncService = SyncServiceImpl._( atClientManager, atClient, notificationService, remoteSecondary); await syncService.statsServiceListener(); diff --git a/packages/at_client/test/test_utils/no_op_services.dart b/packages/at_client/test/test_utils/no_op_services.dart index e16ccc1c6..e686f8a1e 100644 --- a/packages/at_client/test/test_utils/no_op_services.dart +++ b/packages/at_client/test/test_utils/no_op_services.dart @@ -2,16 +2,16 @@ import 'package:at_client/at_client.dart'; class ServiceFactoryWithNoOpServices extends DefaultAtServiceFactory { @override - Future syncService(AtClient atClient, - AtClientManager atClientManager, NotificationService notificationService, - {String? enrollmentId}) async { + Future syncService( + AtClient atClient, + AtClientManager atClientManager, + NotificationService notificationService) async { return NoOpSyncService(); } @override Future notificationService( - AtClient atClient, AtClientManager atClientManager, - {String? enrollmentId}) async { + AtClient atClient, AtClientManager atClientManager) async { return NoOpNotificationService(); } } From ef2b166a790ec49eefc28791c43d48dd3a07ed7d Mon Sep 17 00:00:00 2001 From: murali-shris Date: Thu, 26 Oct 2023 15:48:05 +0530 Subject: [PATCH 3/4] fix: review comments --- packages/at_client/lib/src/client/at_client_impl.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/at_client/lib/src/client/at_client_impl.dart b/packages/at_client/lib/src/client/at_client_impl.dart index 1bdae43dc..4a8927e05 100644 --- a/packages/at_client/lib/src/client/at_client_impl.dart +++ b/packages/at_client/lib/src/client/at_client_impl.dart @@ -114,6 +114,9 @@ class AtClientImpl implements AtClient, AtSignChangeListener { late final AtSignLogger _logger; + @override + String? enrollmentId; + @visibleForTesting static final Map atClientInstanceMap = {}; @@ -991,7 +994,4 @@ class AtClientImpl implements AtClient, AtSignChangeListener { } return result.notificationID; } - - @override - String? enrollmentId; } From 8a488e012a8c415c586a6c1142eed11639a4dfe9 Mon Sep 17 00:00:00 2001 From: murali-shris Date: Thu, 26 Oct 2023 17:15:19 +0530 Subject: [PATCH 4/4] fix: added unit test --- packages/at_client/test/at_client_impl_test.dart | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/at_client/test/at_client_impl_test.dart b/packages/at_client/test/at_client_impl_test.dart index 5d9d24a68..39f9a16a6 100644 --- a/packages/at_client/test/at_client_impl_test.dart +++ b/packages/at_client/test/at_client_impl_test.dart @@ -250,4 +250,16 @@ void main() { expect(key.key, 'uppercase'); //key should be converted to lower case }); }); + + group('A group of tests related to setting enrollmentId', () { + test( + 'A test to verify enrollmentId is set in atClient after calling setCurrentAtSign', + () async { + final testEnrollmentId = 'abc123'; + var atClientManager = await AtClientManager.getInstance() + .setCurrentAtSign('@alice', 'wavi', AtClientPreference(), + enrollmentId: testEnrollmentId); + expect(atClientManager.atClient.enrollmentId, testEnrollmentId); + }); + }); }