SyncServiceImpl.isInSync() does not work with APKAM

[gkc]

Describe the bug

SyncServiceImpl.isInSync() creates a new RemoteSecondary each time it is called. The reasons for doing this are not entirely clear, given that SyncServiceImpl.create() already creates a RemoteSecondary , but that is another story.

In SyncServiceImpl.create() the enrollmentId is passed to the RemoteSecondary constructor:

    remoteSecondary ??= RemoteSecondary(
        atClient.getCurrentAtSign()!, atClient.getPreferences()!,
        atChops: atClient.atChops, enrollmentId: atClient.enrollmentId);

However in SyncServiceImpl.isInSync() it is not:

      remoteSecondary = RemoteSecondary(
          _atClient.getCurrentAtSign()!, _atClient.getPreferences()!,
          atChops: _atClient.atChops);

Authentication will then fail, since the enrollmentId must be supplied in order to authenticate with APKAM keys.

Steps to reproduce

Create a program like this:

import 'dart:io';

import 'package:at_cli_commons/at_cli_commons.dart';

Future<void> main(List<String> args) async {
  try {
    var atClient = (await CLIBase.fromCommandLineArgs(args)).atClient;
    await atClient.syncService.isInSync();
    exit(0);
  } catch (e) {
    if (e is ArgumentError) {
      print(CLIBase.argsParser.usage);
    }
    print(e);
    exit(1);
  }
}

its dependencies are

dependencies:
  at_cli_commons: ^1.3.0
  at_client: ^3.3.0

Running it with a PKAM keys file:

gary@gkc2019 example % dart bin/is_in_sync_example.dart -a @garycasey -n test
Connecting ... Connected
Is in sync? false

Running it with an APKAM keys file:

gary@gkc2019 example % dart bin/is_in_sync_example.dart -a @garycasey -k ~/@garycasey.test.mbp.atKeys -n test
Connecting ... Connected
Exception: Exception: Unable to fetch latest server commit id: Exception: Exception: Failed connecting to @garycasey. error:{"errorCode":"AT0401","errorDescription":"Exception: pkam authentication failed"}

Expected behavior

isInSync should also pass enrollmentId to the RemoteSecondary constructor

Additional context

As part of fixing this I'd also like to see whether we can eliminate isInSync's creation of its own RemoteSecondary

[XavierChanth]

Looks like there should be a private _getRemoteSecondary() function, and each place where we need the remote secondary, we should call something like:

_remoteSecondary ??= _getRemoteSecondary();

[murali-shris]

The reason for adding separate remote secondary for isInSync method.
#404
In the current draft PR , I have removed the logic.
Will try replicating the old isInSync bug with latest code.

[purnimavenkatasubbu]

Test Results

1. . Try replicating old isInSync bug raised by Colin with sync_service_bug branch.
   AtClientManager.synchService.isInSync crashes after a few calls . #404
   Issue is not replicated.

2. replicate the current isInSync issue with sync_service_bug branch

dart sync_test.dart -a @ purnima -n wavi -k /home/purnima/.atsign/keys/@ purnima_buzzkey.atKeys
-d vip.ve.atsign.zone
Connecting ... Connected
Exception: Exception: Unable to fetch latest server commit id: Exception: Exception: Failed connecting to @ purnima. error:{"errorCode":"AT0401","errorDescription":"Exception: pkam authentication failed"}
purnima@ purnima:~/Documents/libraries_sync_bug/at_libraries/packages/at_cli_commons/test$

3. Re run the test with sync_service_bug branch
   dart sync_test.dart -a @ purnima -n wavi -k /home/purnima/.atsign/keys/@purnima_buzzkey.atKeys -d vip.ve.atsign.zone
   Connecting ... Connected
   is inSync : false

4. sanity testing with buzz app and onboarding_cli with my branch

buzzLogs.txt
clitests.txt