fix: Commit log special key bug

packages/at_persistence_secondary_server/CHANGELOG.md

@@ -1,3 +1,5 @@
+## 3.0.65
+- fix: Modified checks in _specialKey method in commit log keystore to match only reserved shared_key and encryption public key.
 ## 3.0.64
 - build[deps]: Upgraded the following packages:
   - at_commons to v5.0.0

packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart

@@ -179,6 +179,9 @@ class CommitLogKeyStore extends BaseCommitLogKeyStore {
     }
   }
 
+  /// match a key to be passed in getEntries/getChanges when these conditions are met
+  /// if enrolledNamespace is passed, key namespace has be in list of enrolled namespace with required authorization
+  /// if regex is passed, key has to match the regex or it has to be a special key.
   bool _acceptKey(String atKey, String regex,
       {List<String>? enrolledNamespace}) {
     return _isNamespaceAuthorised(atKey, enrolledNamespace) &&
@@ -218,11 +221,12 @@ class CommitLogKeyStore extends BaseCommitLogKeyStore {
     return RegExp(regex).hasMatch(atKey);
   }
 
+  /// match only reserved keys which have to be synced from server to client
+  /// e.g @bob:shared_key@alice, shared_key.bob@alice, public:publickey@alice
   bool _isSpecialKey(String atKey) {
-    return atKey.contains(AtConstants.atEncryptionSharedKey) ||
-        atKey.startsWith('public:') ||
-        atKey.contains(AtConstants.atPkamSignature) ||
-        atKey.contains(AtConstants.atSigningPrivateKey);
+    return (atKey.contains(AtConstants.atEncryptionSharedKey) &&
+            RegexUtil.keyType(atKey, false) == KeyType.reservedKey) ||
+        atKey.startsWith(AtConstants.atEncryptionPublicKey);
   }
 
   /// Returns the latest commitEntry of the key.
@@ -564,7 +568,8 @@ class CommitLogCache {
     if (existingCommitId != null &&
         commitEntry.commitId != null &&
         existingCommitId > commitEntry.commitId!) {
-      _logger.shout('Ignoring commit entry update to cache. existingCommitId: $existingCommitId | toUpdateWithCommitId: ${commitEntry.commitId}');
+      _logger.shout(
+          'Ignoring commit entry update to cache. existingCommitId: $existingCommitId | toUpdateWithCommitId: ${commitEntry.commitId}');
       return;
     }
     _updateCacheLog(key, commitEntry);

packages/at_persistence_secondary_server/pubspec.yaml

@@ -1,6 +1,6 @@
 name: at_persistence_secondary_server
 description: A Dart library with the implementation classes for the persistence layer of the secondary server.
-version: 3.0.64
+version: 3.0.65
 repository: https://github.com/atsign-foundation/at_server
 homepage: https://docs.atsign.com/
 
@@ -19,6 +19,14 @@ dependencies:
   at_persistence_spec: ^2.0.14
   meta: ^1.8.0
 
+#TODO replace with published version
+dependency_overrides:
+  at_commons:
+    git:
+      url: https://github.com/atsign-foundation/at_libraries.git
+      ref: special_key_bug
+      path: packages/at_commons
+
 dev_dependencies:
   lints: ^2.0.1
   test: ^1.22.1

packages/at_persistence_secondary_server/test/commit_log_test.dart

@@ -134,12 +134,12 @@ void main() async {
         //loop to create 10 keys - even keys have commitId null - odd keys have commitId
         for (int i = 0; i < 10; i++) {
           if (i % 2 == 0) {
-            await commitLogKeystore.getBox().add(CommitEntry(
+            await commitLogKeystore.add(CommitEntry(
                 'test_key_false_$i.wavi@alice',
                 CommitOp.UPDATE,
                 DateTime.now()));
           } else {
-            await commitLogKeystore.getBox().add(CommitEntry(
+            await commitLogKeystore.add(CommitEntry(
                 'test_key_false_$i.wavi@alice', CommitOp.UPDATE, DateTime.now())
               ..commitId = i);
           }
@@ -250,6 +250,42 @@ void main() async {
         expect(commitId, 0);
         expect(commitLogInstance?.lastCommittedSequenceNumber(), 0);
       });
+      test('test to verify last committed sequenceNumber with regex', () async {
+        var commitLogInstance =
+            await (AtCommitLogManagerImpl.getInstance().getCommitLog('@alice'));
+        await commitLogInstance?.commit(
+            'public:location_1.wavi@alice', CommitOp.UPDATE);
+        await commitLogInstance?.commit(
+            'public:phone.buzz@alice', CommitOp.UPDATE);
+        await commitLogInstance?.commit(
+            'public:location_2.wavi@alice', CommitOp.UPDATE);
+        await commitLogInstance?.commit(
+            'public:email.buzz@alice', CommitOp.UPDATE);
+        expect(
+            await commitLogInstance
+                ?.lastCommittedSequenceNumberWithRegex('buzz'),
+            3);
+        expect(
+            await commitLogInstance
+                ?.lastCommittedSequenceNumberWithRegex('wavi'),
+            2);
+        await commitLogInstance?.commit(
+            'public:location_1.wavi@alice', CommitOp.UPDATE);
+        await commitLogInstance?.commit(
+            'public:location_2.wavi@alice', CommitOp.DELETE);
+        await commitLogInstance?.commit(
+            'public:phone.buzz@alice', CommitOp.DELETE);
+        await commitLogInstance?.commit(
+            'public:email.buzz@alice', CommitOp.DELETE);
+        expect(
+            await commitLogInstance
+                ?.lastCommittedSequenceNumberWithRegex('buzz'),
+            7);
+        expect(
+            await commitLogInstance
+                ?.lastCommittedSequenceNumberWithRegex('wavi'),
+            5);
+      });
     });
     group('A group of commit log compaction tests', () {
       setUp(() async => await setUpFunc(storageDir));
@@ -413,19 +449,18 @@ void main() async {
         var commitLogInstance =
             await (AtCommitLogManagerImpl.getInstance().getCommitLog('@alice'));
         // Inserting commitEntry with commitId 0
-        await commitLogInstance!.commitLogKeyStore.getBox().add(
+        await commitLogInstance!.commitLogKeyStore.add(
             CommitEntry('location@alice', CommitOp.UPDATE, DateTime.now())
               ..commitId = 0);
         // Inserting commitEntry with null commitId
-        await commitLogInstance.commitLogKeyStore.getBox().add(
+        await commitLogInstance.commitLogKeyStore.add(
             CommitEntry('location@alice', CommitOp.UPDATE, DateTime.now()));
         // Inserting commitEntry with commitId 2
-        await commitLogInstance.commitLogKeyStore.getBox().add(
+        await commitLogInstance.commitLogKeyStore.add(
             CommitEntry('phone@alice', CommitOp.UPDATE, DateTime.now())
               ..commitId = 2);
         // Inserting commitEntry with null commitId
         await commitLogInstance.commitLogKeyStore
-            .getBox()
             .add(CommitEntry('mobile@alice', CommitOp.UPDATE, DateTime.now()));
 
         var commitLogMap = await commitLogInstance.commitLogKeyStore.toMap();
@@ -627,10 +662,10 @@ void main() async {
         //loop to create 10 keys - even keys have commitId null - odd keys have commitId
         for (int i = 0; i < 10; i++) {
           if (i % 2 == 0) {
-            await commitLogKeystore.getBox().add(CommitEntry(
+            await commitLogKeystore.add(CommitEntry(
                 'test_key_true_$i', CommitOp.UPDATE, DateTime.now()));
           } else {
-            await commitLogKeystore.getBox().add(
+            await commitLogKeystore.add(
                 CommitEntry('test_key_true_$i', CommitOp.UPDATE, DateTime.now())
                   ..commitId = i);
           }
@@ -671,6 +706,54 @@ void main() async {
             thirdCommitId! > firstCommitId! && thirdCommitId > secondCommitId!);
         expect(commitEntryInCache?.commitId, thirdCommitId);
       });
+      test(
+          'A test to verify only reserved key - shared key is returned in getEntries when namespace is passed',
+          () async {
+        var commitLogInstance =
+            await (AtCommitLogManagerImpl.getInstance().getCommitLog('@alice'));
+        var commitLogKeystore = commitLogInstance!.commitLogKeyStore;
+        await commitLogKeystore.add(CommitEntry(
+            '@bob:shared_key@alice', CommitOp.UPDATE, DateTime.now()));
+        await commitLogKeystore.add(CommitEntry(
+            'shared_key.bob@alice', CommitOp.UPDATE, DateTime.now()));
+        await commitLogKeystore.add(CommitEntry(
+            'bob:test_shared_key@alice', CommitOp.UPDATE, DateTime.now()));
+        Iterator<MapEntry<String, CommitEntry>>? changes =
+            commitLogInstance.commitLogKeyStore.getEntries(-1, regex: '.buzz');
+        while (changes.moveNext()) {
+          final commitEntry = changes.current.value;
+          if (commitEntry.commitId == 0) {
+            expect(commitEntry.atKey, '@bob:shared_key@alice');
+          } else if (commitEntry.commitId == 1) {
+            expect(commitEntry.atKey, 'shared_key.bob@alice');
+          }
+        }
+      });
+      test(
+          'A test to verify non reserved key - shared key is returned in getEntries only when namespace is passed and key namespace matches passed namespace',
+          () async {
+        var commitLogInstance =
+            await (AtCommitLogManagerImpl.getInstance().getCommitLog('@alice'));
+        var commitLogKeystore = commitLogInstance!.commitLogKeyStore;
+        await commitLogKeystore.add(CommitEntry(
+            '@bob:shared_key@alice', CommitOp.UPDATE, DateTime.now()));
+        await commitLogKeystore.add(CommitEntry(
+            'shared_key.bob@alice', CommitOp.UPDATE, DateTime.now()));
+        await commitLogKeystore.add(CommitEntry(
+            'bob:test_shared_key.buzz@alice', CommitOp.UPDATE, DateTime.now()));
+        Iterator<MapEntry<String, CommitEntry>>? changes =
+            commitLogInstance.commitLogKeyStore.getEntries(-1, regex: '.buzz');
+        while (changes.moveNext()) {
+          final commitEntry = changes.current.value;
+          if (commitEntry.commitId == 0) {
+            expect(commitEntry.atKey, '@bob:shared_key@alice');
+          } else if (commitEntry.commitId == 1) {
+            expect(commitEntry.atKey, 'shared_key.bob@alice');
+          } else if (commitEntry.commitId == 2) {
+            expect(commitEntry.atKey, 'bob:test_shared_key.buzz@alice');
+          }
+        }
+      });
     });
     tearDown(() async => await tearDownFunc());
   });

packages/at_secondary_server/pubspec.yaml

@@ -34,6 +34,16 @@ dependencies:
   yaml: 3.1.2
   logging: 1.2.0
 
+#TODO replace with published version
+dependency_overrides:
+  at_persistence_secondary_server:
+    path: ../at_persistence_secondary_server
+  at_commons:
+    git:
+      url: https://github.com/atsign-foundation/at_libraries.git
+      ref: special_key_bug
+      path: packages/at_commons
+
 dev_dependencies:
   build_runner: ^2.3.3
   test: ^1.24.4

packages/at_secondary_server/test/sync_unit_test.dart

@@ -491,17 +491,12 @@ void main() {
 
         List syncResponse = jsonDecode(response.data!);
 
-        expect(syncResponse.length, 2);
-        // As per design, all the public keys are not filtered when matching with regex.
-        expect(syncResponse[0]['atKey'], 'public:country.wavi@alice');
-        expect(syncResponse[0]['commitId'], 1);
+        expect(syncResponse.length, 1);
+
+        expect(syncResponse[0]['atKey'], 'firstname.buzz@alice');
+        expect(syncResponse[0]['commitId'], 3);
         expect(syncResponse[0]['operation'], '+');
         expect(syncResponse[0]['metadata']['version'], '0');
-
-        expect(syncResponse[1]['atKey'], 'firstname.buzz@alice');
-        expect(syncResponse[1]['commitId'], 3);
-        expect(syncResponse[1]['operation'], '+');
-        expect(syncResponse[1]['metadata']['version'], '0');
       });
       test('test to verify sync response does not exceed the buffer limit',
           () async {