feat: Sync skip deletes <do not merge> #2131
Conversation
murali-shris
changed the title
| @@ -127,12 +127,13 @@ class AtCommitLog extends BaseAtCommitLog { | |||
| /// Returns the Iterator of [_commitLogCacheMap] from the commitId specified. | |||
| @server | |||
| Iterator<MapEntry<String, CommitEntry>> getEntries(int commitId, | |||
| {String? regex, int limit = 25}) { | |||
| {String? regex, int limit = 25, bool skipDeletes = false}) { | |||
There was a problem hiding this comment.
I think we can make this a strategy.. where FetchAllKeysStrategy is to get all and SkipDeletesStrategy is to skip deletes, instead of if-else code might look more readable.
| @@ -43,7 +43,8 @@ class SyncProgressiveVerbHandler extends AbstractVerbHandler { | |||
| // Get entries to sync | |||
| var commitEntryIterator = atCommitLog!.getEntries( | |||
| int.parse(verbParams[AtConstants.fromCommitSequence]!) + 1, | |||
| regex: verbParams['regex']); | |||
| regex: verbParams['regex'], | |||
| skipDeletes: verbParams['skipDeletes'] == 'true'); | |||
There was a problem hiding this comment.
VerbSyntax also needs this parameter, otherwise regex validation would fail.
There was a problem hiding this comment.
yes. those changes are in at_commons
https://github.com/atsign-foundation/at_libraries/pull/696/files
| @override | ||
| bool shouldIncludeEntryInSyncResponse( | ||
| CommitEntry commitEntry, int commitId, String regex, | ||
| {List<String>? enrolledNamespace}) { |
There was a problem hiding this comment.
Dont we need call to super.shouldIncludeEntryInSyncResponse()?
There was a problem hiding this comment.
i kept the method in base class abstract
| import 'package:at_persistence_secondary_server/at_persistence_secondary_server.dart'; | ||
| import 'package:at_utils/at_utils.dart'; | ||
|
|
||
| abstract class SyncKeysFetchStrategy { |
There was a problem hiding this comment.
Should we call it NamespaceHonoringKeyFetchStrategy?
There was a problem hiding this comment.
Honoring namespace is one of the checks done. regex match and skipping delete commits are the other two checks. In the future if we have any other checks not related to namespace we may have to rename this class
| @@ -840,6 +840,33 @@ void main() async { | |||
| expect(commitEntriesMap.containsKey('public:phone.wavi@alice'), false); | |||
| expect(commitEntriesMap.containsKey('public:location@alice'), true); | |||
| }); | |||
There was a problem hiding this comment.
Please add another test which both uses a regex and sets skipDeletes: true
| @@ -469,6 +469,39 @@ void main() { | |||
| expect(syncResponse[3]['operation'], '*'); | |||
| }); | |||
|
|
|||
There was a problem hiding this comment.
Please add another test which both uses a regex and sets skipDeletes: true
…ion/at_server into sync_skip_deletes
Please enter the commit message for your changes. Lines starting
- What I did
- How I did it
- How to verify it
TODO:
publish at_commons and at_persistence changes.