fix: at_commons 4.0.0 uptake in at_Client #1192

purnimavenkatasubbu · 2024-01-04T05:58:16Z

- What I did
Updated the following dependencies

at_commons to v4.0.0
at_utils to v3.0.16
at_lookup to v3.0.44
at_chops to v1.0.7
at_persistence_secondary_server to v3.0.60

Also made updates as per the at_commons:4.0.0 changes

Updated the melos.yaml temporarily to avoid at_client being picked from the path. This causes "dart run melos bootstrap" to fail because at_client_mobile is compatible with at_commons-4.0.0 while at_client is upgraded to 4.0.0. This does not affect the mobile apps. Once the at_client_mobile is compatible with at_commons-4.0.0 will revert the change.

- How to verify it
All the unit, functional, and end2end tests should pass
All the apps should work fine

- Description for the changelog

…ndencies # Conflicts: # packages/at_client/CHANGELOG.md

…to update_atclient_dependencies

gkc

This PR has a lot of files changed

Via the "Files Changed" tab), please add a brief explanation of the reason for each change (at least the first time it appears - e.g. the change from atKey.key ?? '' to atKey.key appears in many place - please explain why.)

Please also consider for every change here whether it is a breaking change for the at_client package

sitaram-kalluri · 2024-01-09T06:32:12Z

packages/at_client/lib/src/at_collection/impl/collection_methods_impl.dart

@@ -41,7 +41,7 @@ class AtCollectionMethodImpl {
    _logger.finest('Self key to be used : $atKey');
    var atOperationItemStatus = AtOperationItemStatus(
        atSign: atKey.sharedBy ?? '',
-        key: atKey.key ?? '',
+        key: atKey.key,


Earlier, the AtKey.key is a nullable variable, but the named argument "key" does not accept NULL value. Hence defaulted to empty string in case of null. In at_commons-4.0.0, the AtKey.key is prefixed with late and "?" (which denotes that variable can be null) is removed because key can never be null in AtKey,

sitaram-kalluri · 2024-01-09T06:39:32Z

packages/at_client/lib/src/at_collection/impl/default_key_maker.dart

-      ..metadata!.ccd = objectLifeCycleOptions?.cascadeDelete ?? true
-      ..metadata!.ttl = objectLifeCycleOptions?.timeToLive?.inMilliseconds
-      ..metadata!.ttb = objectLifeCycleOptions?.timeToBirth?.inMilliseconds
+      ..metadata.ccd = objectLifeCycleOptions?.cascadeDelete ?? true


In at_commons-> AtKey, updated the "metadata" field from nullable field (removed "?"). With the change, the field is defaulted to Metadata instance - "Metadata metadata = Metadata();"

sitaram-kalluri · 2024-01-09T07:16:55Z

packages/at_client/lib/src/client/at_client_impl.dart

-      ..sharedWith = atKey.sharedWith
-      ..atKey = keyWithNamespace
-      ..sharedBy = atKey.sharedBy;
+    var builder = DeleteVerbBuilder()..atKey = atKey;


The fields of AtKey are now substituted with AtKey instances in all verb builders. The atKey object is directly assigned, and the code responsible for assigning fields of atKey has been removed

sitaram-kalluri · 2024-01-09T07:17:38Z

packages/at_client/lib/src/client/at_client_impl.dart

@@ -470,7 +451,7 @@ class AtClientImpl implements AtClient, AtSignChangeListener {

  @visibleForTesting
  ensureLowerCase(AtKey atKey) {
-    if ((atKey.key != null && upperCaseRegex.hasMatch(atKey.key!)) ||
+    if (upperCaseRegex.hasMatch(atKey.key) ||


Since Atkey.key is modified as a non null variable in at_commons, removed the non-null assertion operator

sitaram-kalluri · 2024-01-09T07:18:28Z

packages/at_client/lib/src/client/at_client_impl.dart

@@ -487,12 +468,12 @@ class AtClientImpl implements AtClient, AtSignChangeListener {
    if (atKey.sharedBy.isNull) {
      atKey.sharedBy = _atSign;
    }
-    if (atKey.metadata!.namespaceAware) {
+    if (atKey.metadata.namespaceAware) {


Since Atkey.metadata is modified as a non null variable (defaulted to "Metadata()"), removed the non-null assertion operator

sitaram-kalluri · 2024-01-09T07:19:37Z

packages/at_client/lib/src/client/at_client_impl.dart

-      ..isBinary = metadata.isBinary
-      ..isEncrypted = metadata.isEncrypted
-      ..dataSignature = metadata.dataSignature
+      ..atKey = atKey


The fields of AtKey are now substituted with AtKey instances in all verb builders. The atKey object is directly assigned, and the code responsible for assigning fields of atKey has been removed

sitaram-kalluri · 2024-01-09T07:20:01Z

packages/at_client/lib/src/client/at_client_impl.dart

      ..operation = AtConstants.updateMeta;

    var updateMetaResult = await getSecondary()
-        .executeVerb(builder, sync: SyncUtil.shouldSync(updateKey!));
+        .executeVerb(builder, sync: SyncUtil.shouldSync(updateKey));


Since Atkey.key is modified as a non null variable in at_commons, removed the non-null assertion operator

sitaram-kalluri · 2024-01-09T07:22:00Z

packages/at_client/lib/src/client/at_client_impl.dart

@@ -949,7 +920,7 @@ class AtClientImpl implements AtClient, AtSignChangeListener {
      PriorityEnum? priority,
      StrategyEnum? strategy,
      int? latestN,
-      String? notifier = SYSTEM,
+      String? notifier = AtConstants.system,


Replaced the use of legacy at_constants (which are now removed from at_commons -4.0.0) with new references to new AtConstants file

sitaram-kalluri · 2024-01-09T07:25:43Z

packages/at_client/lib/src/client/verb_builder_manager.dart

-        (atKey.metadata != null &&
-            atKey.metadata!.isPublic! &&
-            !atKey.metadata!.isCached)) {
+        (atKey.metadata.isPublic && !atKey.metadata.isCached)) {


Since AtKey.metadata is not null and boolean fields in Metadata have default values instead of null, removed the null check and non null assertion operator

sitaram-kalluri · 2024-01-09T07:31:39Z

packages/at_client/lib/src/manager/sync_manager.dart

-        (metaData[CCD].toLowerCase() == 'true')
-            ? builder.ccd = true
-            : builder.ccd = false;
+      if (metaData[AtConstants.ttl] != null)


The AT_TTL, AT_TTB, AT_TTR...which is are a part of "at_constants_legacy.dart" which is deprecated and removed in at_commons-4.0.0. The "at_constants_legacy.dart" is replaced with "at_constants.dart" in at_commons.

sitaram-kalluri · 2024-01-09T07:33:06Z

packages/at_client/lib/src/transformer/request_transformer/get_request_transformer.dart

@@ -12,8 +12,6 @@ class GetRequestTransformer implements RequestTransformer<AtKey, VerbBuilder> {

  @override
  VerbBuilder transform(AtKey atKey, {RequestOptions? requestOptions}) {
-    // Set the default metadata if not already set.
-    atKey.metadata ??= Metadata();


Since AtKey.metadata is defaulted to "Metadata()". Hence removed the above line.

sitaram-kalluri · 2024-01-09T07:33:21Z

packages/at_client/lib/src/transformer/request_transformer/notify_request_transformer.dart

@@ -21,8 +21,6 @@ class NotificationRequestTransformer
  @override
  Future<NotifyVerbBuilder> transform(
      NotificationParams notificationParams) async {
-    // If metadata is not set, initialize Metadata instance.
-    notificationParams.atKey.metadata ??= Metadata();


Since AtKey.metadata is defaulted to "Metadata()". Hence removed the above line.

sitaram-kalluri · 2024-01-09T07:35:20Z

packages/at_client/lib/src/transformer/request_transformer/notify_request_transformer.dart

-    builder.ivNonce = notificationParams.atKey.metadata?.ivNonce;
-    builder.skeEncKeyName = notificationParams.atKey.metadata?.skeEncKeyName;
-    builder.skeEncAlgo = notificationParams.atKey.metadata?.skeEncAlgo;
+    builder.atKey.metadata.ttl = notificationParams.atKey.metadata.ttl;


The fields of AtKey and Metadata are now substituted with AtKey instances in all verb builders. The metadata values are assigned to the metadata fields in the atKey.metadata.

sitaram-kalluri · 2024-01-09T07:36:54Z

packages/at_client/lib/src/transformer/response_transformer/get_response_transformer.dart

-    if (tuple.one.metadata != null &&
-        tuple.one.metadata!.isBinary != null &&
-        tuple.one.metadata!.isBinary!) {
+    if (tuple.one.metadata.isBinary) {


Since in AtKey.metadata is defaulted to "Metadata()" and boolean fields in metadata have default values, removed null check.

sitaram-kalluri · 2024-01-09T07:41:40Z

...es/at_client/lib/src/transformer/response_transformer/notification_response_transformer.dart

-      ..sharedBy = atNotification.from
-      ..metadata = atNotification.metadata;
+      ..sharedBy = atNotification.from;
+    if (atNotification.metadata != null) {


Since AtKey.metadata does not hold null value, added a null check before assigning the metadata.

…to update_atclient_dependencies

sitaram-kalluri · 2024-01-09T07:46:26Z

This PR has a lot of files changed

Via the "Files Changed" tab), please add a brief explanation of the reason for each change (at least the first time it appears - e.g. the change from atKey.key ?? '' to atKey.key appears in many place - please explain why.)

Please also consider for every change here whether it is a breaking change for the at_client package

@gkc: Added comments explaining the changes. We have ensured not to introduce any breaking changes. @purnimavenkatasubbu has completed a round of testing with mobile apps with this branch and no bugs are reported.

gkc · 2024-01-09T12:45:44Z

packages/at_client/lib/src/client/at_client_impl.dart

-    atKey.metadata ??= Metadata();
-    String keyWithNamespace;
-    if (atKey.metadata!.namespaceAware) {
-      keyWithNamespace = AtClientUtil.getKeyWithNameSpace(atKey, _preference!);


Why is the namespace fixing no longer required?

By assigning atKey directly to the verb builder, key construction utilizes AtKey.toString(), appending the namespace to the key. For the key we do not want namespace to be appended, leaving the namespace fields unset or initializing them to an empty string will suffice.

However, when the user doesn't set a namespace for the AtKey, the earlier practice involved appending the namespace from the AtClientPreferences when namespaceAware is set to "true" is missing here. Will add the code where if atKey.namespace is null or empty and namespaceAware is true, then set the namespace from AtClientPreference.namespace

gkc · 2024-01-09T12:54:08Z

packages/at_client/lib/src/transformer/request_transformer/put_request_transformer.dart

-      ..isBinary =
-          (atKey.metadata?.isBinary != null) ? atKey.metadata?.isBinary! : false
-      ..isLocal = atKey.isLocal;
+      ..atKey = (AtKey()


Why are you creating a new atKey object here? Can't we just use the one that was passed as a method parameter?

The AtKey received from the put method is assigned to the UpdateVerbBuilder.atKey. Removed the above because we not have assign each field of atKey to the atKey in UpdateVerbBuilder.

gkc

A few more requests and comments

gkc · 2024-01-09T12:58:12Z

This PR has a lot of files changed
Via the "Files Changed" tab), please add a brief explanation of the reason for each change (at least the first time it appears - e.g. the change from atKey.key ?? '' to atKey.key appears in many place - please explain why.)
Please also consider for every change here whether it is a breaking change for the at_client package

@gkc: Added comments explaining the changes. We have ensured not to introduce any breaking changes. @purnimavenkatasubbu has completed a round of testing with mobile apps with this branch and no bugs are reported.

Thank you both

This reverts commit 57cb917.

…ace from at_client preferences and assign AtKey to UpdateVerbBuilder directly

…to update_atclient_dependencies

gkc

@sitaram-kalluri @purnimavenkatasubbu Please can you do another round of testing of atmospherePro, atBuzz and at_talk with this branch, just to make sure

sitaram-kalluri · 2024-01-12T11:33:39Z

@sitaram-kalluri @purnimavenkatasubbu Please can you do another round of testing of atmospherePro, atBuzz and at_talk with this branch, just to make sure

@gkc : Sure, will do a round of testing with apps and keep you updated.

purnimavenkatasubbu · 2024-01-12T17:36:27Z

I did a quick round of sanity testing with atMospherePro, atBuzz, and atTalk pointing to this branch of at_client. It looks good. Attaching below POTs
atBuzzLogsUpdateAtClientDependencies12th.txt
[atmospherePRoUpdateAtClientDependencies12Jan.txt](https://github.com/atsign-foundation/at_client_sdk/files/13922366/atmospherePRoUpda

teAtClientDependencies12Jan.txt)

purnimavenkatasubbu and others added 8 commits January 3, 2024 20:18

at_commons 4.0.0 uptake in at_Client

1de43a3

resolved merge conflicts

82ec47f

fixed tests

85cbb24

Merge remote-tracking branch 'origin/trunk' into update_atclient_depe…

4343fe5

…ndencies # Conflicts: # packages/at_client/CHANGELOG.md

fix: Update the version in at_client

12ff4c9

Merge branch 'trunk' into update_atclient_dependencies

ed3b11d

fix: Remove at_client in melos.yaml

ebe4008

Merge remote-tracking branch 'origin/update_atclient_dependencies' in…

7aa87ac

…to update_atclient_dependencies

sitaram-kalluri requested review from sitaram-kalluri, gkc and murali-shris and removed request for sitaram-kalluri January 8, 2024 06:40

sitaram-kalluri marked this pull request as ready for review January 8, 2024 11:33

Merge branch 'trunk' into update_atclient_dependencies

504c9c0

gkc reviewed Jan 8, 2024

View reviewed changes

sitaram-kalluri reviewed Jan 9, 2024

View reviewed changes

sitaram-kalluri added 2 commits January 9, 2024 13:13

fix: Update the null check condition in shared_key_decryption.dart

1c20bce

Merge remote-tracking branch 'origin/update_atclient_dependencies' in…

cef5f61

…to update_atclient_dependencies

sitaram-kalluri requested a review from gkc January 9, 2024 07:44

gkc reviewed Jan 9, 2024

View reviewed changes

gkc requested changes Jan 9, 2024

View reviewed changes

murali-shris and others added 5 commits January 10, 2024 16:35

feat: replace encryption util encryption methods with at_chops

57cb917

Revert "feat: replace encryption util encryption methods with at_chops"

495696a

This reverts commit 57cb917.

fix: In delete method when atkey.namespace is unset default to namesp…

68aa6f4

…ace from at_client preferences and assign AtKey to UpdateVerbBuilder directly

Merge remote-tracking branch 'origin/update_atclient_dependencies' in…

894dc2f

…to update_atclient_dependencies

Merge branch 'trunk' into update_atclient_dependencies

341a0a8

sitaram-kalluri requested a review from gkc January 11, 2024 10:22

gkc requested changes Jan 12, 2024

View reviewed changes

gkc previously approved these changes Jan 13, 2024

View reviewed changes

resolved merge conflicts

03e9ab0

purnimavenkatasubbu dismissed gkc’s stale review via 03e9ab0 January 16, 2024 06:40

purnimavenkatasubbu requested review from gkc and sitaram-kalluri January 16, 2024 09:26

gkc approved these changes Jan 16, 2024

View reviewed changes

gkc merged commit 4d67c5b into trunk Jan 16, 2024
8 checks passed

gkc deleted the update_atclient_dependencies branch January 16, 2024 09:31

gkc mentioned this pull request Jan 16, 2024

fix: Uptake at_commons 4.0.0 changes #1179

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: at_commons 4.0.0 uptake in at_Client #1192

fix: at_commons 4.0.0 uptake in at_Client #1192

purnimavenkatasubbu commented Jan 4, 2024 •

edited by sitaram-kalluri

Loading

gkc left a comment

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri Jan 9, 2024

sitaram-kalluri commented Jan 9, 2024

gkc Jan 9, 2024

sitaram-kalluri Jan 10, 2024

gkc Jan 9, 2024

sitaram-kalluri Jan 11, 2024

gkc left a comment

gkc commented Jan 9, 2024

gkc left a comment

sitaram-kalluri commented Jan 12, 2024

purnimavenkatasubbu commented Jan 12, 2024

fix: at_commons 4.0.0 uptake in at_Client #1192

fix: at_commons 4.0.0 uptake in at_Client #1192

Conversation

purnimavenkatasubbu commented Jan 4, 2024 • edited by sitaram-kalluri Loading

gkc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sitaram-kalluri commented Jan 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkc left a comment

Choose a reason for hiding this comment

gkc commented Jan 9, 2024

gkc left a comment

Choose a reason for hiding this comment

sitaram-kalluri commented Jan 12, 2024

purnimavenkatasubbu commented Jan 12, 2024

purnimavenkatasubbu commented Jan 4, 2024 •

edited by sitaram-kalluri

Loading