Skip to content

Commit

Permalink
Merge pull request #2115 from atsign-foundation/commit_log_special_ke…
Browse files Browse the repository at this point in the history 
…y_bug

fix: Commit log special key bug
  • Loading branch information
@gkc
gkc authored Oct 16, 2024
2 parents f298b04 + e670ee8 commit 8338b6d
Show file tree
Hide file tree
Showing 6 changed files with 234 additions and 43 deletions.
2 changes: 2 additions & 0 deletions packages/at_persistence_secondary_server/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down
37 changes: 22 additions & 15 deletions packages/at_persistence_secondary_server/lib/src/log/commitlog/commit_log_keystore.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class CommitLogKeyStore extends BaseCommitLogKeyStore {
Future<int?> lastCommittedSequenceNumberWithRegex(String regex,
{List<String>? 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 =
Expand Down Expand Up @@ -179,10 +179,13 @@ class CommitLogKeyStore extends BaseCommitLogKeyStore {
}
}

bool _acceptKey(String atKey, String regex,
/// 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 _shouldIncludeKeyInSyncResponse(String atKey, String regex,
{List<String>? enrolledNamespace}) {
return _isNamespaceAuthorised(atKey, enrolledNamespace) &&
(_isRegexMatches(atKey, regex) || _isSpecialKey(atKey));
(_keyMatchesRegex(atKey, regex) || _alwaysIncludeInSync(atKey));
}

bool _isNamespaceAuthorised(
Expand Down Expand Up @@ -214,15 +217,18 @@ class CommitLogKeyStore extends BaseCommitLogKeyStore {
return false;
}

bool _isRegexMatches(String atKey, String regex) {
bool _keyMatchesRegex(String atKey, String regex) {
return RegExp(regex).hasMatch(atKey);
}

bool _isSpecialKey(String atKey) {
return atKey.contains(AtConstants.atEncryptionSharedKey) ||
atKey.startsWith('public:') ||
atKey.contains(AtConstants.atPkamSignature) ||
atKey.contains(AtConstants.atSigningPrivateKey);
/// 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) ||
(atKey.startsWith('public:') && !atKey.contains('.'));
}

/// Returns the latest commitEntry of the key.
Expand All @@ -238,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;
}
Expand Down Expand Up @@ -420,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;
}
}
Expand All @@ -446,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);
Expand Down Expand Up @@ -483,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) {
Expand Down Expand Up @@ -564,7 +570,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);
Expand Down
10 changes: 9 additions & 1 deletion packages/at_persistence_secondary_server/pubspec.yaml
View file
Original file line number Diff line number Diff line change
@@ -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/

Expand All @@ -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
Expand Down
205 changes: 187 additions & 18 deletions packages/at_persistence_secondary_server/test/commit_log_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -250,6 +250,88 @@ 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);
});
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));
Expand Down Expand Up @@ -413,19 +495,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();
Expand Down Expand Up @@ -627,28 +708,23 @@ 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(
'test_key_true_$i', CommitOp.UPDATE, DateTime.now()));
await commitLogKeystore.add(CommitEntry(
'test_key_true_$i@alice', CommitOp.UPDATE, DateTime.now()));
} else {
await commitLogKeystore.getBox().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<MapEntry<String, CommitEntry>>? changes =
commitLogInstance.commitLogKeyStore.getEntries(-1);
//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',
Expand All @@ -671,6 +747,99 @@ 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');
Map<String?, CommitEntry> commitEntriesMap = {};
while (changes.moveNext()) {
final commitEntry = changes.current.value;
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 {
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');
Map<String?, CommitEntry> commitEntriesMap = {};
while (changes.moveNext()) {
final commitEntry = changes.current.value;
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<MapEntry<String, CommitEntry>>? changes =
commitLogInstance.commitLogKeyStore.getEntries(-1);
Map<String?, CommitEntry> 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<MapEntry<String, CommitEntry>>? changes =
commitLogInstance.commitLogKeyStore.getEntries(-1, regex: '.buzz');
Map<String?, CommitEntry> 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());
});
Expand Down
Loading

0 comments on commit 8338b6d

Please sign in to comment.