Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: skip reserved keys from sync conflict #1065

Merged
merged 7 commits into from
Jun 15, 2023
Merged

fix: skip reserved keys from sync conflict #1065

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
158dcfc
fix: skip reserved keys from sync conflict
sitaram-kalluri Jun 14, 2023
e3de0d9
fix: Modify the IF condition with regular expression
sitaram-kalluri Jun 15, 2023
5baeced
Merge branch 'trunk' into srk_sync_conflict_fix
gkc Jun 15, 2023
4ba5158
Merge remote-tracking branch 'origin/trunk' into srk_sync_conflict_fix
sitaram-kalluri Jun 15, 2023
aecdf4f
fix: Modify encryptedSharedKey matching Regular expression and add un…
sitaram-kalluri Jun 15, 2023
d6f0162
fix: Run dart formatter
sitaram-kalluri Jun 15, 2023
825e8b0
Merge branch 'trunk' into srk_sync_conflict_fix
gkc Jun 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions packages/at_client/lib/src/service/sync_service_impl.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,13 +569,14 @@ class SyncServiceImpl implements SyncService, AtSignChangeListener {
}

Future<ConflictInfo?> _setConflictInfo(final serverCommitEntry) async {
final key = serverCommitEntry['atKey'];
String key = serverCommitEntry['atKey'];
// publickey.<atsign>@<currentatsign> is used to store the public key of
// other atsign. The value is not encrypted.
// The keys starting with publickey. and shared_key. are the reserved keys
// The keys starting with publickey. and keys that contain shared_key
// (@someone:shared_key@me, shared_key.someone@me) are the reserved keys
// and do not require actions. Hence skipping from checking conflict resolution.
if (key.startsWith('publickey.') ||
key.startsWith('shared_key.') ||
key.startsWith(RegExp('shared_key.+@.+|@.+shared_key@.+')) ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shared_key.+@.+ will match shared_keyyyyyyy@alice
@.+shared_key@.+ will match @ssssssshared_key:phone.wavi@alice

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the Regular expression and added unit tests.

key.startsWith('cached:')) {
_logger.finer('$key found in conflict resolution, returning null');
return null;
Expand All @@ -587,7 +588,14 @@ class SyncServiceImpl implements SyncService, AtSignChangeListener {
}
final conflictInfo = ConflictInfo();
try {
final localAtValue = await _atClient.get(atKey);
AtValue localAtValue;
// For a conflicting key, if an uncommitted entry is of CommitOp.Delete, then
// key will not exist in Key-Store. On KeyNotFoundException, return null.
try {
localAtValue = await _atClient.get(atKey);
} on KeyNotFoundException {
return null;
}
if (atKey is PublicKey || key.contains('public:')) {
final serverValue = serverCommitEntry['value'];
if (localAtValue.value != serverValue) {
Expand Down
107 changes: 86 additions & 21 deletions packages/at_client/test/sync_new_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import 'package:at_client/src/util/sync_util.dart';
import 'package:at_commons/at_builders.dart';
import 'package:at_persistence_secondary_server/at_persistence_secondary_server.dart';
import 'package:at_persistence_secondary_server/src/keystore/hive_keystore.dart';
import 'package:at_utils/at_logger.dart';
import 'package:crypton/crypton.dart';
import 'package:mocktail/mocktail.dart';
import 'package:test/test.dart';
Expand Down Expand Up @@ -2768,7 +2767,6 @@ void main() {
test(
'A test to verify when sync conflict info when key present in uncommitted entries and in server response of sync',
() async {
AtSignLogger.root_level = 'finer';
// ------------------------------ Setup ----------------------------------
LocalSecondary? localSecondary = LocalSecondary(mockAtClient,
keyStore: TestResources.getHiveKeyStore(TestResources.atsign));
Expand Down Expand Up @@ -2810,7 +2808,7 @@ void main() {
'"metadata":{"createdAt":"2022-11-07 13:42:02.703Z"},'
'"commitId":2,"operation":"*"}'
','
'{"atKey":"@alice:conflict_shared_key.demo${TestResources.atsign}",'
'{"atKey":"@alice:shared_key${TestResources.atsign}",'
'"value":"shared_key_remote_value",'
'"metadata":{"createdAt":"2022-11-07 13:42:02.703Z","isEncrypted":"true"},'
'"commitId":3,"operation":"*"}]'));
Expand All @@ -2830,8 +2828,8 @@ void main() {
AtKey.fromString('public:conflict_key1${TestResources.atsign}')))
.thenAnswer(
(invocation) => Future.value(AtValue()..value = 'local_value'));
when(() => mockAtClient.get(AtKey.fromString(
'@alice:conflict_shared_key.demo${TestResources.atsign}')))
when(() => mockAtClient.get(
AtKey.fromString('@alice:shared_key${TestResources.atsign}')))
.thenAnswer((invocation) =>
Future.value(AtValue()..value = 'shared_key_local_value'));
when(() => mockAtKeyDecryptionManager.get(
Expand All @@ -2845,8 +2843,7 @@ void main() {
await localSecondary.putValue(
'public:conflict_key1${TestResources.atsign}', 'localValue');
await localSecondary.putValue(
'@alice:conflict_shared_key.demo${TestResources.atsign}',
'shared_key_local_value');
'@alice:shared_key${TestResources.atsign}', 'shared_key_local_value');
CustomSyncProgressListener progressListener =
CustomSyncProgressListener();
syncService.addProgressListener(progressListener);
Expand All @@ -2863,11 +2860,10 @@ void main() {
expect(keyInfo.conflictInfo?.remoteValue, 'remote_value');
expect(keyInfo.conflictInfo?.localValue, 'local_value');
}
if (keyInfo.key == '@alice:conflict_shared_key.demo@hiro' &&
// Since shared_key is a reserved key, setting conflict info is skipped
if (keyInfo.key == '@alice:shared_key@hiro' &&
keyInfo.syncDirection == SyncDirection.remoteToLocal) {
expect(
keyInfo.conflictInfo?.remoteValue, 'shared_key_remote_value');
expect(keyInfo.conflictInfo?.localValue, 'shared_key_local_value');
expect(keyInfo.conflictInfo, null);
}
}
}));
Expand All @@ -2878,7 +2874,6 @@ void main() {
test(
'A test to verify conflict info sets errorOrExceptionMessage when exception occurs in setConflictInfo',
() async {
AtSignLogger.root_level = 'finer';
// ------------------------------ Setup ----------------------------------
LocalSecondary? localSecondary = LocalSecondary(mockAtClient,
keyStore: TestResources.getHiveKeyStore(TestResources.atsign));
Expand Down Expand Up @@ -2910,8 +2905,8 @@ void main() {
any(that: SyncVerbBuilderMatcher()),
sync: any(
named: "sync"))).thenAnswer((invocation) => Future.value('data:['
'{"atKey":"@alice:conflict_shared_key.demo${TestResources.atsign}",'
'"value":"shared_key_remote_value",'
'{"atKey":"@alice:conflict_phone_key.demo${TestResources.atsign}",'
'"value":"phone_key_remote_value",'
'"metadata":{"createdAt":"2022-11-07 13:42:02.703Z","isEncrypted":"true"},'
'"commitId":3,"operation":"*"}]'));
when(() => mockRemoteSecondary.executeCommand(any(),
Expand All @@ -2926,20 +2921,20 @@ void main() {
.thenAnswer((invocation) =>
throw AtKeyNotFoundException('key is not found in keystore'));
when(() => mockAtClient.get(AtKey.fromString(
'@alice:conflict_shared_key.demo${TestResources.atsign}')))
'@alice:conflict_phone_key.demo${TestResources.atsign}')))
.thenAnswer((invocation) =>
Future.value(AtValue()..value = 'shared_key_local_value'));
Future.value(AtValue()..value = 'phone_key_local_value'));
when(() => mockAtKeyDecryptionManager.get(
any(that: ConflictKeyMatcher()), TestResources.atsign))
.thenAnswer((_) => mockSharedKeyDecryption);
when(() => mockSharedKeyDecryption.decrypt(
any(that: ConflictKeyMatcher()), 'shared_key_remote_value'))
any(that: ConflictKeyMatcher()), 'phone_key_remote_value'))
.thenAnswer((_) => throw AtPublicKeyNotFoundException(
'Encryption public key not found'));

await localSecondary.putValue(
'@alice:conflict_shared_key.demo${TestResources.atsign}',
'shared_key_local_value');
'@alice:conflict_phone_key.demo${TestResources.atsign}',
'phone_key_local_value');
CustomSyncProgressListener progressListener =
CustomSyncProgressListener();
syncService.addProgressListener(progressListener);
Expand All @@ -2951,10 +2946,80 @@ void main() {
.listen(expectAsync1((syncProgress) {
expect(syncProgress.syncStatus, SyncStatus.success);
for (KeyInfo keyInfo in syncProgress.keyInfoList!) {
if (keyInfo.key == '@alice:conflict_shared_key.demo@hiro' &&
if (keyInfo.key == '@alice:conflict_phone_key.demo@hiro' &&
keyInfo.syncDirection == SyncDirection.remoteToLocal) {
expect(keyInfo.conflictInfo?.errorOrExceptionMessage,
'Exception occurred when setting conflict info for @alice:conflict_shared_key.demo@hiro | Exception: Encryption public key not found');
'Exception occurred when setting conflict info for @alice:conflict_phone_key.demo@hiro | Exception: Encryption public key not found');
}
}
}));
});

test(
'A test to verify conflict info when uncommitted entry has a delete operation',
() async {
LocalSecondary? localSecondary = LocalSecondary(mockAtClient,
keyStore: TestResources.getHiveKeyStore(TestResources.atsign));

SyncServiceImpl syncService = await SyncServiceImpl.create(mockAtClient,
atClientManager: mockAtClientManager,
notificationService: mockNotificationService,
remoteSecondary: mockRemoteSecondary) as SyncServiceImpl;
syncService.syncUtil = SyncUtil(atCommitLog: TestResources.commitLog);

registerFallbackValue(FakeSyncVerbBuilder());
registerFallbackValue(FakeUpdateVerbBuilder());
registerFallbackValue(FakeAtKey());

when(() => mockNetworkUtil.isNetworkAvailable())
.thenAnswer((_) => Future.value(true));
when(() => mockAtClient.getLocalSecondary()).thenReturn(localSecondary);
when(() => mockRemoteSecondary
.executeVerb(any(that: StatsVerbBuilderMatcher())))
.thenAnswer((invocation) => Future.value('data:[{"value":"3"}]'));
when(() => mockAtClient.getPreferences())
.thenAnswer((_) => AtClientPreference());
when(() => mockRemoteSecondary.executeVerb(
any(that: SyncVerbBuilderMatcher()),
sync: any(
named: "sync"))).thenAnswer((invocation) => Future.value('data:['
'{"atKey":"@alice:conflict-key${TestResources.atsign}",'
'"value":"remote_value",'
'"metadata":{"createdAt":"2022-11-07 13:42:02.703Z","isEncrypted":"true"},'
'"commitId":3,"operation":"*"}]'));
when(() => mockRemoteSecondary.executeCommand(any(),
auth: any(named: "auth")))
.thenAnswer((invocation) =>
Future.value('data:[{"id":1,"response":{"data":"3"}}]'));
when(() => mockAtClient.put(
any(that: LastReceivedServerCommitIdMatcher()), any()))
.thenAnswer((_) => Future.value(true));
when(() =>
mockAtClient.get(any(that: LastReceivedServerCommitIdMatcher())))
.thenAnswer((invocation) =>
throw AtKeyNotFoundException('key is not found in keystore'));
when(() => mockAtClient.get(any(that: ConflictKeyMatcher()))).thenAnswer(
(invocation) =>
throw KeyNotFoundException('key is not found in keystore'));

await localSecondary.executeVerb(DeleteVerbBuilder()
..sharedWith = '@alice'
..atKey = 'conflict-key'
..sharedBy = TestResources.atsign);
CustomSyncProgressListener progressListener =
CustomSyncProgressListener();
syncService.addProgressListener(progressListener);
syncService.sync();
await syncService.processSyncRequests(
respectSyncRequestQueueSizeAndRequestTriggerDuration: false);

progressListener.streamController.stream
.listen(expectAsync1((syncProgress) {
expect(syncProgress.syncStatus, SyncStatus.success);
for (KeyInfo keyInfo in syncProgress.keyInfoList!) {
if (keyInfo.key == '@alice:conflict-key@hiro' &&
keyInfo.syncDirection == SyncDirection.remoteToLocal) {
expect(keyInfo.conflictInfo == null, true);
}
}
}));
Expand Down