From 3ea9883d002650d5cd4230fece8aa8684558115c Mon Sep 17 00:00:00 2001 From: Murali Date: Tue, 8 Oct 2024 22:15:54 +0530 Subject: [PATCH 01/10] fix: special key bug fix --- .../log/commitlog/commit_log_keystore.dart | 5 +-- .../test/commit_log_test.dart | 36 +++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart b/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart index e1e6990b7..06b60bd06 100644 --- a/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart +++ b/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart @@ -220,7 +220,7 @@ class CommitLogKeyStore extends BaseCommitLogKeyStore { bool _isSpecialKey(String atKey) { return atKey.contains(AtConstants.atEncryptionSharedKey) || - atKey.startsWith('public:') || + atKey.startsWith(AtConstants.atEncryptionPublicKey) || atKey.contains(AtConstants.atPkamSignature) || atKey.contains(AtConstants.atSigningPrivateKey); } @@ -564,7 +564,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); diff --git a/packages/at_persistence_secondary_server/test/commit_log_test.dart b/packages/at_persistence_secondary_server/test/commit_log_test.dart index bb1973743..e89110a9d 100644 --- a/packages/at_persistence_secondary_server/test/commit_log_test.dart +++ b/packages/at_persistence_secondary_server/test/commit_log_test.dart @@ -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)); From e42136a77e69d0b17fcd5526cb6d91856aed9059 Mon Sep 17 00:00:00 2001 From: Murali Date: Tue, 8 Oct 2024 22:17:28 +0530 Subject: [PATCH 02/10] fix: pubspec change to test persistence in server --- packages/at_secondary_server/pubspec.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/at_secondary_server/pubspec.yaml b/packages/at_secondary_server/pubspec.yaml index 9e9a8e47d..62915c1a7 100644 --- a/packages/at_secondary_server/pubspec.yaml +++ b/packages/at_secondary_server/pubspec.yaml @@ -34,6 +34,11 @@ 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 + dev_dependencies: build_runner: ^2.3.3 test: ^1.24.4 From 9ddbacbdd328385ed5573fce81b51a6a1109f824 Mon Sep 17 00:00:00 2001 From: Murali Date: Tue, 8 Oct 2024 22:36:36 +0530 Subject: [PATCH 03/10] fix: sync unit test assertion change --- .../at_secondary_server/test/sync_unit_test.dart | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/at_secondary_server/test/sync_unit_test.dart b/packages/at_secondary_server/test/sync_unit_test.dart index ddca654b9..568fcbb4b 100644 --- a/packages/at_secondary_server/test/sync_unit_test.dart +++ b/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 { From 1a09a6d0bc6ff55d68a78296224f48cb25ac04b8 Mon Sep 17 00:00:00 2001 From: Murali Date: Wed, 9 Oct 2024 11:28:21 +0530 Subject: [PATCH 04/10] fix: changes to publish persistence --- packages/at_persistence_secondary_server/CHANGELOG.md | 2 ++ packages/at_persistence_secondary_server/pubspec.yaml | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/at_persistence_secondary_server/CHANGELOG.md b/packages/at_persistence_secondary_server/CHANGELOG.md index 0c3fb52c3..a2fff66f1 100644 --- a/packages/at_persistence_secondary_server/CHANGELOG.md +++ b/packages/at_persistence_secondary_server/CHANGELOG.md @@ -1,3 +1,5 @@ +## 3.0.65 +- fix: Modified check for public encryption key in _specialKey method in commit log keystore ## 3.0.64 - build[deps]: Upgraded the following packages: - at_commons to v5.0.0 diff --git a/packages/at_persistence_secondary_server/pubspec.yaml b/packages/at_persistence_secondary_server/pubspec.yaml index 2a09b650d..18a7dbd37 100644 --- a/packages/at_persistence_secondary_server/pubspec.yaml +++ b/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/ From 719ab2b8466cd322f9fb582e6c6f191e3dd48d96 Mon Sep 17 00:00:00 2001 From: Murali Date: Tue, 15 Oct 2024 11:50:09 +0530 Subject: [PATCH 05/10] fix: add at_commons dependency overrides --- .../lib/src/log/commitlog/commit_log_keystore.dart | 7 +++---- packages/at_persistence_secondary_server/pubspec.yaml | 7 +++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart b/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart index 06b60bd06..2e2b75b07 100644 --- a/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart +++ b/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart @@ -219,10 +219,9 @@ class CommitLogKeyStore extends BaseCommitLogKeyStore { } bool _isSpecialKey(String atKey) { - return atKey.contains(AtConstants.atEncryptionSharedKey) || - atKey.startsWith(AtConstants.atEncryptionPublicKey) || - 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. diff --git a/packages/at_persistence_secondary_server/pubspec.yaml b/packages/at_persistence_secondary_server/pubspec.yaml index 18a7dbd37..abde62d79 100644 --- a/packages/at_persistence_secondary_server/pubspec.yaml +++ b/packages/at_persistence_secondary_server/pubspec.yaml @@ -19,6 +19,13 @@ dependencies: at_persistence_spec: ^2.0.14 meta: ^1.8.0 +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 From 011c9f0f573d9c6f2e7e9a4935045103ec5aa2c2 Mon Sep 17 00:00:00 2001 From: Murali Date: Tue, 15 Oct 2024 11:55:56 +0530 Subject: [PATCH 06/10] fix: add at_commons dependency overrides --- packages/at_secondary_server/pubspec.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/at_secondary_server/pubspec.yaml b/packages/at_secondary_server/pubspec.yaml index 62915c1a7..a3386a7ad 100644 --- a/packages/at_secondary_server/pubspec.yaml +++ b/packages/at_secondary_server/pubspec.yaml @@ -38,6 +38,11 @@ dependencies: 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 From 17190dd2002b970e4ec4b5e70771426ef32252d4 Mon Sep 17 00:00:00 2001 From: Murali Date: Tue, 15 Oct 2024 13:14:21 +0530 Subject: [PATCH 07/10] fix: added test and doc for _acceptKey --- .../log/commitlog/commit_log_keystore.dart | 5 ++ .../test/commit_log_test.dart | 63 ++++++++++++++++--- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart b/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart index 2e2b75b07..06f62f50e 100644 --- a/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart +++ b/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? enrolledNamespace}) { return _isNamespaceAuthorised(atKey, enrolledNamespace) && @@ -218,6 +221,8 @@ 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) && RegexUtil.keyType(atKey, false) == KeyType.reservedKey) || diff --git a/packages/at_persistence_secondary_server/test/commit_log_test.dart b/packages/at_persistence_secondary_server/test/commit_log_test.dart index e89110a9d..318303a46 100644 --- a/packages/at_persistence_secondary_server/test/commit_log_test.dart +++ b/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); } @@ -449,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(); @@ -663,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); } @@ -707,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>? 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>? 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()); }); From 498c51cd314bb22c1897acae6df094daedde0704 Mon Sep 17 00:00:00 2001 From: Murali Date: Tue, 15 Oct 2024 13:44:34 +0530 Subject: [PATCH 08/10] fix: modified pubspec and change --- packages/at_persistence_secondary_server/CHANGELOG.md | 2 +- packages/at_persistence_secondary_server/pubspec.yaml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/at_persistence_secondary_server/CHANGELOG.md b/packages/at_persistence_secondary_server/CHANGELOG.md index a2fff66f1..eafb7390a 100644 --- a/packages/at_persistence_secondary_server/CHANGELOG.md +++ b/packages/at_persistence_secondary_server/CHANGELOG.md @@ -1,5 +1,5 @@ ## 3.0.65 -- fix: Modified check for public encryption key in _specialKey method in commit log keystore +- 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 diff --git a/packages/at_persistence_secondary_server/pubspec.yaml b/packages/at_persistence_secondary_server/pubspec.yaml index abde62d79..a3f03e095 100644 --- a/packages/at_persistence_secondary_server/pubspec.yaml +++ b/packages/at_persistence_secondary_server/pubspec.yaml @@ -19,6 +19,7 @@ dependencies: at_persistence_spec: ^2.0.14 meta: ^1.8.0 +#TODO replace with published version dependency_overrides: at_commons: git: From 69ec2c2f27bfd12d883e2589a5a095e2d9a83885 Mon Sep 17 00:00:00 2001 From: Murali Date: Wed, 16 Oct 2024 13:28:18 +0530 Subject: [PATCH 09/10] fix: added test for public key without namespace --- .../log/commitlog/commit_log_keystore.dart | 28 ++-- .../test/commit_log_test.dart | 132 +++++++++++++++--- 2 files changed, 124 insertions(+), 36 deletions(-) diff --git a/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart b/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart index 06f62f50e..6d42559f1 100644 --- a/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart +++ b/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart @@ -61,7 +61,7 @@ class CommitLogKeyStore extends BaseCommitLogKeyStore { Future lastCommittedSequenceNumberWithRegex(String regex, {List? enrolledNamespace}) async { var lastCommittedEntry = (getBox() as Box).values.lastWhere( - (entry) => (_acceptKey(entry.atKey, regex, + (entry) => (_shouldIncludeKeyInSyncResponse(entry.atKey, regex, enrolledNamespace: enrolledNamespace)), orElse: () => NullCommitEntry()); var lastCommittedSequenceNum = @@ -182,10 +182,11 @@ 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, + bool _shouldIncludeKeyInSyncResponse(String atKey, String regex, {List? enrolledNamespace}) { return _isNamespaceAuthorised(atKey, enrolledNamespace) && - (_isRegexMatches(atKey, regex) || _isSpecialKey(atKey)); + (_doesKeyMatchRegex(atKey, regex) || + _shouldKeyBeIncludedInSyncIfNoRegexMatch(atKey)); } bool _isNamespaceAuthorised( @@ -217,16 +218,17 @@ class CommitLogKeyStore extends BaseCommitLogKeyStore { return false; } - bool _isRegexMatches(String atKey, String regex) { + bool _doesKeyMatchRegex(String atKey, String regex) { 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) { + /// match only reserved keys and public keys without namespace which have to be synced from server to client, if regex doesn't match + /// e.g @bob:shared_key@alice, shared_key.bob@alice, public:publickey@alice, public:phone@alice + bool _shouldKeyBeIncludedInSyncIfNoRegexMatch(String atKey) { return (atKey.contains(AtConstants.atEncryptionSharedKey) && RegexUtil.keyType(atKey, false) == KeyType.reservedKey) || - atKey.startsWith(AtConstants.atEncryptionPublicKey); + atKey.startsWith(AtConstants.atEncryptionPublicKey) || + (atKey.startsWith('public:') && !atKey.contains('.')); } /// Returns the latest commitEntry of the key. @@ -242,7 +244,7 @@ class CommitLogKeyStore extends BaseCommitLogKeyStore { .entriesList() .where((element) => element.value.commitId! >= commitId && - _acceptKey(element.value.atKey!, regex)) + _shouldIncludeKeyInSyncResponse(element.value.atKey!, regex)) .take(limit); return commitEntriesIterable.iterator; } @@ -424,7 +426,7 @@ class ClientCommitLogKeyStore extends CommitLogKeyStore { // Iterate through the regex's in the _lastSyncedEntryCacheMap. // Updates the commitEntry against the matching regexes. for (var regex in _lastSyncedEntryCacheMap.keys) { - if (_acceptKey(commitEntry.atKey!, regex)) { + if (_shouldIncludeKeyInSyncResponse(commitEntry.atKey!, regex)) { _lastSyncedEntryCacheMap[regex] = commitEntry; } } @@ -450,7 +452,7 @@ class ClientCommitLogKeyStore extends CommitLogKeyStore { limit ??= values.length + 1; for (CommitEntry element in values) { if (element.key >= startKey && - _acceptKey(element.atKey!, regexString) && + _shouldIncludeKeyInSyncResponse(element.atKey!, regexString) && changes.length <= limit) { if (element.commitId == null) { changes.add(element); @@ -487,8 +489,8 @@ class ClientCommitLogKeyStore extends CommitLogKeyStore { // Returns the commitEntry with maximum commitId matching the given regex. // otherwise returns NullCommitEntry lastSyncedEntry = values.lastWhere( - (entry) => - (_acceptKey(entry!.atKey!, regex) && (entry.commitId != null)), + (entry) => (_shouldIncludeKeyInSyncResponse(entry!.atKey!, regex) && + (entry.commitId != null)), orElse: () => NullCommitEntry()); if (lastSyncedEntry == null || lastSyncedEntry is NullCommitEntry) { diff --git a/packages/at_persistence_secondary_server/test/commit_log_test.dart b/packages/at_persistence_secondary_server/test/commit_log_test.dart index 318303a46..3f92ab455 100644 --- a/packages/at_persistence_secondary_server/test/commit_log_test.dart +++ b/packages/at_persistence_secondary_server/test/commit_log_test.dart @@ -286,6 +286,52 @@ void main() async { ?.lastCommittedSequenceNumberWithRegex('wavi'), 5); }); + test( + 'A test to verify lastCommittedSequenceNumber does not include key which does not match regex', + () async { + var commitLogInstance = + await (AtCommitLogManagerImpl.getInstance().getCommitLog('@alice')); + var commitLogKeystore = commitLogInstance!.commitLogKeyStore; + await commitLogKeystore.add(CommitEntry( + 'public:phone.buzz@alice', CommitOp.UPDATE, DateTime.now())); + await commitLogKeystore.add(CommitEntry( + 'public:email.wavi@alice', CommitOp.UPDATE, DateTime.now())); + final lastCommittedSeq = await commitLogKeystore + .lastCommittedSequenceNumberWithRegex('.buzz'); + expect(lastCommittedSeq, 0); + }); + test( + 'A test to verify lastCommittedSequenceNumber include key which matches regex and enrollednamespace', + () async { + var commitLogInstance = + await (AtCommitLogManagerImpl.getInstance().getCommitLog('@alice')); + var commitLogKeystore = commitLogInstance!.commitLogKeyStore; + await commitLogKeystore.add( + CommitEntry('phone.buzz@alice', CommitOp.UPDATE, DateTime.now())); + await commitLogKeystore.add( + CommitEntry('phone.wavi@alice', CommitOp.UPDATE, DateTime.now())); + await commitLogKeystore.add(CommitEntry( + 'location.buzz@alice', CommitOp.UPDATE, DateTime.now())); + final lastCommittedSeq = await commitLogKeystore + .lastCommittedSequenceNumberWithRegex('.buzz', + enrolledNamespace: ['buzz']); + expect(lastCommittedSeq, 2); + }); + test( + 'A test to verify lastCommittedSequenceNumber does not include key which does not match enrollednamespace', + () async { + var commitLogInstance = + await (AtCommitLogManagerImpl.getInstance().getCommitLog('@alice')); + var commitLogKeystore = commitLogInstance!.commitLogKeyStore; + await commitLogKeystore.add( + CommitEntry('phone.buzz@alice', CommitOp.UPDATE, DateTime.now())); + await commitLogKeystore.add( + CommitEntry('phone.wavi@alice', CommitOp.UPDATE, DateTime.now())); + final lastCommittedSeq = await commitLogKeystore + .lastCommittedSequenceNumberWithRegex('.*', + enrolledNamespace: ['buzz']); + expect(lastCommittedSeq, 0); + }); }); group('A group of commit log compaction tests', () { setUp(() async => await setUpFunc(storageDir)); @@ -663,11 +709,11 @@ void main() async { for (int i = 0; i < 10; i++) { if (i % 2 == 0) { await commitLogKeystore.add(CommitEntry( - 'test_key_true_$i', CommitOp.UPDATE, DateTime.now())); + 'test_key_true_$i@alice', CommitOp.UPDATE, DateTime.now())); } else { - await commitLogKeystore.add( - CommitEntry('test_key_true_$i', CommitOp.UPDATE, DateTime.now()) - ..commitId = i); + await commitLogKeystore.add(CommitEntry( + 'test_key_true_$i@alice', CommitOp.UPDATE, DateTime.now()) + ..commitId = i); } } Iterator>? changes = @@ -675,15 +721,10 @@ void main() async { //run loop to ensure all commit entries have been returned; irrespective of commitId null or not int i = 0; while (changes.moveNext()) { - if (i % 2 == 0) { - //while creation of commit entries, even keys have been set with commitId == null - expect(changes.current.value.commitId, null); - } else { - //while creation of commit entries, even keys have been set with commitId equal to iteration count - expect(changes.current.value.commitId, i); - } + expect(changes.current.value.commitId, i); i++; } + expect(i, 10); }); test( 'verify that CommitEntry with higher commitId is retained in cache for the same key', @@ -720,15 +761,17 @@ void main() async { 'bob:test_shared_key@alice', CommitOp.UPDATE, DateTime.now())); Iterator>? changes = commitLogInstance.commitLogKeyStore.getEntries(-1, regex: '.buzz'); + Map commitEntriesMap = {}; 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'); - } + commitEntriesMap[commitEntry.atKey] = commitEntry; } + expect(commitEntriesMap.containsKey('@bob:shared_key@alice'), true); + expect(commitEntriesMap.containsKey('shared_key.bob@alice'), true); + expect( + commitEntriesMap.containsKey('bob:test_shared_key@alice'), false); }); + 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 { @@ -743,16 +786,59 @@ void main() async { 'bob:test_shared_key.buzz@alice', CommitOp.UPDATE, DateTime.now())); Iterator>? changes = commitLogInstance.commitLogKeyStore.getEntries(-1, regex: '.buzz'); + Map commitEntriesMap = {}; 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'); - } + commitEntriesMap[commitEntry.atKey] = commitEntry; + } + expect(commitEntriesMap.containsKey('@bob:shared_key@alice'), true); + expect(commitEntriesMap.containsKey('shared_key.bob@alice'), true); + expect(commitEntriesMap.containsKey('bob:test_shared_key.buzz@alice'), + true); + }); + test( + 'A test to verify public key without namespace is returned in getEntries when no regex is passed', + () async { + var commitLogInstance = + await (AtCommitLogManagerImpl.getInstance().getCommitLog('@alice')); + var commitLogKeystore = commitLogInstance!.commitLogKeyStore; + await commitLogKeystore.add( + CommitEntry('public:phone@alice', CommitOp.UPDATE, DateTime.now())); + await commitLogKeystore.add( + CommitEntry('public:email@alice', CommitOp.UPDATE, DateTime.now())); + Iterator>? changes = + commitLogInstance.commitLogKeyStore.getEntries(-1); + Map commitEntriesMap = {}; + while (changes.moveNext()) { + final commitEntry = changes.current.value; + commitEntriesMap[commitEntry.atKey] = commitEntry; + } + expect(commitEntriesMap.containsKey('public:phone@alice'), true); + expect(commitEntriesMap.containsKey('public:email@alice'), true); + }); + + test( + 'A test to verify public keys with matched namespace and no namespace is returned in getEntries when regex is passed', + () async { + var commitLogInstance = + await (AtCommitLogManagerImpl.getInstance().getCommitLog('@alice')); + var commitLogKeystore = commitLogInstance!.commitLogKeyStore; + await commitLogKeystore.add(CommitEntry( + 'public:phone.buzz@alice', CommitOp.UPDATE, DateTime.now())); + await commitLogKeystore.add(CommitEntry( + 'public:email.wavi@alice', CommitOp.UPDATE, DateTime.now())); + await commitLogKeystore.add(CommitEntry( + 'public:location@alice', CommitOp.UPDATE, DateTime.now())); + Iterator>? changes = + commitLogInstance.commitLogKeyStore.getEntries(-1, regex: '.buzz'); + Map commitEntriesMap = {}; + while (changes.moveNext()) { + final commitEntry = changes.current.value; + commitEntriesMap[commitEntry.atKey] = commitEntry; } + expect(commitEntriesMap.containsKey('public:phone.buzz@alice'), true); + expect(commitEntriesMap.containsKey('public:phone.wavi@alice'), false); + expect(commitEntriesMap.containsKey('public:location@alice'), true); }); }); tearDown(() async => await tearDownFunc()); From 3ea497fa77429bfc9d0aa201ac5c65ecb0a105df Mon Sep 17 00:00:00 2001 From: Murali Date: Wed, 16 Oct 2024 17:46:01 +0530 Subject: [PATCH 10/10] fix: rename methods --- .../lib/src/log/commitlog/commit_log_keystore.dart | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart b/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart index 6d42559f1..b5a7c98ea 100644 --- a/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart +++ b/packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart @@ -185,8 +185,7 @@ class CommitLogKeyStore extends BaseCommitLogKeyStore { bool _shouldIncludeKeyInSyncResponse(String atKey, String regex, {List? enrolledNamespace}) { return _isNamespaceAuthorised(atKey, enrolledNamespace) && - (_doesKeyMatchRegex(atKey, regex) || - _shouldKeyBeIncludedInSyncIfNoRegexMatch(atKey)); + (_keyMatchesRegex(atKey, regex) || _alwaysIncludeInSync(atKey)); } bool _isNamespaceAuthorised( @@ -218,13 +217,14 @@ class CommitLogKeyStore extends BaseCommitLogKeyStore { return false; } - bool _doesKeyMatchRegex(String atKey, String regex) { + bool _keyMatchesRegex(String atKey, String regex) { return RegExp(regex).hasMatch(atKey); } - /// match only reserved keys and public keys without namespace which have to be synced from server to client, if regex doesn't match - /// e.g @bob:shared_key@alice, shared_key.bob@alice, public:publickey@alice, public:phone@alice - bool _shouldKeyBeIncludedInSyncIfNoRegexMatch(String atKey) { + /// match keys which have to included in sync irrespective of whether regex matches + /// e.g @bob:shared_key@alice, shared_key.bob@alice, public:publickey@alice, + /// public:phone@alice (public key without namespace) + bool _alwaysIncludeInSync(String atKey) { return (atKey.contains(AtConstants.atEncryptionSharedKey) && RegexUtil.keyType(atKey, false) == KeyType.reservedKey) || atKey.startsWith(AtConstants.atEncryptionPublicKey) ||