fix: review comments

atsign-foundation · Nov 20, 2024 · 8d3613d · 8d3613d
1 parent e352240
commit 8d3613d
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 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
@@ -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<KeyInfo> 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<int?> 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
@@ -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
@@ -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);
   });
 }