From 8d3613d3f64ff80aeae3ce9db5a53a3a3034994c Mon Sep 17 00:00:00 2001 From: Murali Date: Wed, 20 Nov 2024 11:35:50 +0530 Subject: [PATCH] fix: review comments --- .../lib/src/service/sync_service_impl.dart | 33 +++++++------ packages/at_client/test/sync_new_test.dart | 47 +++++++++++++++++++ .../test/at_client_sync_test.dart | 8 ++-- 3 files changed, 69 insertions(+), 19 deletions(-) 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 0e2d02046..47f802883 100644 --- a/packages/at_client/lib/src/service/sync_service_impl.dart +++ b/packages/at_client/lib/src/service/sync_service_impl.dart @@ -25,8 +25,6 @@ class SyncServiceImpl implements SyncService, AtSignChangeListener { syncRequestTriggerInSeconds = 3, syncRunIntervalSeconds = 5, queueSize = 5; - //#TODO move to config - static const int initialSyncDelta = 10; late final AtClient _atClient; late final RemoteSecondary _remoteSecondary; late final NotificationServiceImpl _statsNotificationListener; @@ -447,19 +445,9 @@ class SyncServiceImpl implements SyncService, AtSignChangeListener { // server has delete commit entry and the key is not present on local keystore List keyInfoList = []; try { - int? skipDeletesUntil; - if (localCommitIdBeforeSync == -1) { - skipDeletesUntil = serverCommitId; - await _atClient.put( - _skipDeletesUntilCommitId, skipDeletesUntil.toString()); - } else { - try { - skipDeletesUntil = - int.parse((await _atClient.get(_skipDeletesUntilCommitId)).value); - } on AtKeyNotFoundException { - // do nothing - } - } + int? skipDeletesUntil = + await putSkipDeletesUntil(localCommitIdBeforeSync, serverCommitId); + while (serverCommitId > lastReceivedServerCommitId) { _sendTelemetry('_syncFromServer.whileLoop', { "serverCommitId": serverCommitId, @@ -594,6 +582,21 @@ class SyncServiceImpl implements SyncService, AtSignChangeListener { return syncResponseJson; } + @visibleForTesting + Future putSkipDeletesUntil( + int? localCommitIdBeforeSync, int serverCommitId) async { + if (localCommitIdBeforeSync == -1) { + await _atClient.put(_skipDeletesUntilCommitId, serverCommitId.toString()); + return serverCommitId; + } + try { + return int.parse((await _atClient.get(_skipDeletesUntilCommitId)).value); + } on AtKeyNotFoundException { + // do nothing + } + return null; + } + bool _shouldSkipDeletes(int? skipDeletesUntil, int serverCommitId) { if (skipDeletesUntil == null) { return false; diff --git a/packages/at_client/test/sync_new_test.dart b/packages/at_client/test/sync_new_test.dart index 4fb670d34..c3b95320f 100644 --- a/packages/at_client/test/sync_new_test.dart +++ b/packages/at_client/test/sync_new_test.dart @@ -4371,6 +4371,53 @@ void main() { .thenAnswer((invocation) => throw AtKeyNotFoundException('key is not found in keystore')); }); + test( + 'A test to verify putSkipDeletes returns server commitID when localCommitId is -1', + () async { + SyncServiceImpl syncServiceImpl = await SyncServiceImpl.create( + mockAtClient, + atClientManager: mockAtClientManager, + notificationService: mockNotificationService, + remoteSecondary: mockRemoteSecondary) as SyncServiceImpl; + when(() => mockAtClient.put(any(that: SkipDeletesUntilMatcher()), any())) + .thenAnswer((_) => Future.value(true)); + int? skipDeletesUntil = + await syncServiceImpl.putSkipDeletesUntil(-1, 100); + expect(skipDeletesUntil, 100); + }); + test( + 'A test to verify putSkipDeletes returns stored skipDeletesUntil when localCommitId is greater than -1', + () async { + SyncServiceImpl syncServiceImpl = await SyncServiceImpl.create( + mockAtClient, + atClientManager: mockAtClientManager, + notificationService: mockNotificationService, + remoteSecondary: mockRemoteSecondary) as SyncServiceImpl; + when(() => mockAtClient.put(any(that: SkipDeletesUntilMatcher()), any())) + .thenAnswer((_) => Future.value(true)); + when(() => mockAtClient.get(any(that: SkipDeletesUntilMatcher()))) + .thenAnswer((_) => Future.value(AtValue()..value = '40')); + int? skipDeletesUntil = + await syncServiceImpl.putSkipDeletesUntil(25, 100); + expect(skipDeletesUntil, 40); + }); + test( + 'A test to verify putSkipDeletes returns null when localCommitId is greater than -1 and skipDeletesUntil is not stored in local secondary', + () async { + SyncServiceImpl syncServiceImpl = await SyncServiceImpl.create( + mockAtClient, + atClientManager: mockAtClientManager, + notificationService: mockNotificationService, + remoteSecondary: mockRemoteSecondary) as SyncServiceImpl; + when(() => mockAtClient.put(any(that: SkipDeletesUntilMatcher()), any())) + .thenAnswer((_) => Future.value(true)); + when(() => mockAtClient.get(any(that: SkipDeletesUntilMatcher()))) + .thenAnswer((invocation) => + throw AtKeyNotFoundException('key is not found in keystore')); + int? skipDeletesUntil = + await syncServiceImpl.putSkipDeletesUntil(25, 100); + expect(skipDeletesUntil, null); + }); test('A test to verify skip deletes is passed when localCommitId is -1', () async { //----------------- setup----------------- diff --git a/tests/at_functional_test/test/at_client_sync_test.dart b/tests/at_functional_test/test/at_client_sync_test.dart index 976f92b55..4e70bb119 100644 --- a/tests/at_functional_test/test/at_client_sync_test.dart +++ b/tests/at_functional_test/test/at_client_sync_test.dart @@ -276,7 +276,7 @@ void main() { }); test( - 'Verify delete keys are not synced to server when skipDeletesUntil is set', + 'Verify delete keys are not synced from server when skipDeletesUntil is set', () async { var atClient = atClientManager.atClient; var serverCommitId = await SyncUtil() @@ -347,7 +347,7 @@ void main() { await atClient.getRemoteSecondary()!.executeVerb(deleteVerbBuilder); } - // update te keys in buzz namespace + // update the keys in buzz namespace for (int i = 5; i < 7; i++) { var value = 'alice.linkedin$i'; var atKey = @@ -378,6 +378,7 @@ void main() { } var syncVerbBuilder = SyncVerbBuilder() ..skipDeletesUntil = (serverCommitId! + 25) + ..regex = '.wavi' ..commitId = serverCommitId ..isPaginated = true ..limit = 15; @@ -399,7 +400,6 @@ void main() { expect(commitMap.containsKey('public:linkedin_$i.wavi$atSign'), false); } // last delete entry should NOT be present due to namespace buzz not matching - // TODO verify this checks since regex doesnt' match - expect(commitMap.containsKey('public:linkedin_9.buzz$atSign'), true); + expect(commitMap.containsKey('public:linkedin_9.buzz$atSign'), false); }); }