Merge pull request #2099 from atsign-foundation/sync_get_entries_bug

fix: LatestCommitEntryOfEachKey bug due to commit log getEntries default limit 25
atsign-foundation · Sep 25, 2024 · 19da367 · 19da367
2 parents 0664a9d + 2597142
commit 19da367
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 8 deletions.
diff --git a/packages/at_secondary_server/CHANGELOG.md b/packages/at_secondary_server/CHANGELOG.md
@@ -2,6 +2,7 @@
 - feat: Introduce option to unrevoke revoked enrollments
 - feat: Introduce option to delete enrollments that are denied/revoked
 - build[deps]: update dependency versions of at_commons -> 4.1.2, at_utils -> 3.0.18, at_lookup -> 3.0.48
+- fix: LatestCommitEntryOfEachKey metric fixed to return commit log entries till last commitID instead of default limit 25.
 ## 3.0.50
 - fix: Enhance namespace authorisation check to verify when namespace has a period in it
 ## 3.0.49

diff --git a/packages/at_secondary_server/lib/src/verb/metrics/metrics_impl.dart b/packages/at_secondary_server/lib/src/verb/metrics/metrics_impl.dart
@@ -493,14 +493,19 @@ class LatestCommitEntryOfEachKey implements MetricProvider {
     var atCommitLog = await (AtCommitLogManagerImpl.getInstance()
         .getCommitLog(AtSecondaryServerImpl.getInstance().currentAtSign));
 
-    Iterator commitEntryIterator = atCommitLog!.getEntries(-1, regex: regex);
-
-    while (commitEntryIterator.moveNext()) {
-      CommitEntry commitEntry = commitEntryIterator.current.value;
-      responseMap[commitEntry.atKey!] = [
-        commitEntry.commitId,
-        commitEntry.operation.name
-      ];
+    int? lastCommitId = atCommitLog?.lastCommittedSequenceNumber();
+    int lastCommitIdReceived = -1;
+    while (lastCommitId != null && lastCommitIdReceived != lastCommitId) {
+      Iterator commitEntryIterator =
+          atCommitLog!.getEntries(lastCommitIdReceived, regex: regex);
+      while (commitEntryIterator.moveNext()) {
+        CommitEntry commitEntry = commitEntryIterator.current.value;
+        responseMap[commitEntry.atKey!] = [
+          commitEntry.commitId,
+          commitEntry.operation.name
+        ];
+        lastCommitIdReceived = commitEntry.commitId!;
+      }
     }
     return jsonEncode(responseMap);
   }

diff --git a/packages/at_secondary_server/test/stats_verb_test.dart b/packages/at_secondary_server/test/stats_verb_test.dart
@@ -1,5 +1,6 @@
 import 'dart:convert';
 import 'dart:io';
+import 'dart:math';
 
 import 'package:at_persistence_secondary_server/at_persistence_secondary_server.dart';
 import 'package:at_secondary/src/caching/cache_manager.dart';
@@ -471,6 +472,41 @@ void main() {
       expect(latestCommitIdMap['@alice:deletekey-$randomString@alice'][1], '-');
     });
 
+    test(
+        'A test to validate commit entries when commit log entry count is greater than default sync buffer zie',
+        () async {
+      secondaryPersistenceStore = SecondaryPersistenceStoreFactory.getInstance()
+          .getSecondaryPersistenceStore(alice);
+      LastCommitIDMetricImpl.getInstance().atCommitLog =
+          secondaryPersistenceStore!.getSecondaryKeyStore()!.commitLog;
+      var lastCommitId =
+          await LastCommitIDMetricImpl.getInstance().getMetrics();
+      print('lastCommitId before op: $lastCommitId');
+      var randomString = Uuid().v4();
+      int phoneNumber = 1234;
+      int min = 5;
+      int max = 100;
+      // generate a random integer between 5 and 100
+      int randomNumber = min + Random().nextInt(max - min) + 1;
+      for (int i = 1; i <= randomNumber; i++) {
+        phoneNumber = phoneNumber + i;
+        await secondaryPersistenceStore!.getSecondaryKeyStore()!.put(
+            '@alice:phone-${randomString}_$i@alice',
+            AtData()..data = phoneNumber.toString());
+      }
+      lastCommitId = await LastCommitIDMetricImpl.getInstance().getMetrics();
+      var latestCommitIdForEachKey =
+          await LatestCommitEntryOfEachKey().getMetrics();
+      Map<String, dynamic> latestCommitIdMap =
+          jsonDecode(latestCommitIdForEachKey);
+      for (int i = 1; i <= randomNumber; i++) {
+        expect(
+            latestCommitIdMap
+                .containsKey('@alice:phone-${randomString}_$i@alice'),
+            true);
+      }
+    });
+
     test(
         'A test to verify latest commitId among enrolled namespaces is returned',
         () async {
@@ -556,6 +592,18 @@ void main() {
           await LastCommitIDMetricImpl.getInstance().getMetrics(regex: 'buzz');
       expect(lastCommitId, '2');
     });
+    test('A test to check LatestCommitEntryOfEachKey for empty commit log',
+        () async {
+      secondaryPersistenceStore = SecondaryPersistenceStoreFactory.getInstance()
+          .getSecondaryPersistenceStore(alice);
+      LastCommitIDMetricImpl.getInstance().atCommitLog =
+          secondaryPersistenceStore!.getSecondaryKeyStore()!.commitLog;
+      var latestCommitIdForEachKey =
+          await LatestCommitEntryOfEachKey().getMetrics();
+      Map<String, dynamic> latestCommitIdMap =
+          jsonDecode(latestCommitIdForEachKey);
+      expect(latestCommitIdMap.isEmpty, true);
+    });
     tearDown(() async => await verbTestsTearDown());
   });
 }