fix: git#865 - fixes notifying the switch atSign event multiple times

[sitaram-kalluri]

- What I did

• Remove the inactive listeners from AtClientManager._changeListeners list.
• Add unit tests to assert the changes.
• (gkc) Some further simplifications to code associated with switching atSigns

- How I did it

• Added removeChangeListener method in AtClientManager, that removes the given instance of AtSignChangeListener from AtClientManager._changeListeners list
• In AtClientManager._notifyListeners added a line that clones the _changeListeners list and iterate on the cloned list to avoid the concurrent modification exception.
• Some changes to simplify code
  • fix: Added check to SwitchAtSignEvent constructor to prevent events being created when the atClients are the same
  • debug: Added a logger to AtClientManager. Log INFO when switching atSigns
  • refactor: Removed instance variable _previousAtClient from AtClientManager. Instead, determine previousAtClient within the setCurrentAtSign method
  • fix: Added duplicate check to AtClientManager.listenToAtSignChange
  • fix: Changed _currentAtClient from late to being nullable to prevent uninitialized check in setCurrentAtSign
  • fix: AtClientManager now calls _notifyListeners before creating the new NotificationServiceImpl and SyncServiceImpl. This allows those classes' implementations of listenToAtSignChange to be simplified
  • refactor: Removed static maps of instances in NotificationServiceImpl and SyncServiceImpl thus allowing some code simplification
  • refactor: Simplified NotificationServiceImpl.listenToAtSignChange
  • refactor: Simplified SyncServiceImpl.listenToAtSignChange, and added code to close the RemoteSecondary's AtLookUp's connection if there is one

- How to verify it

• On the switch atSign event, AtClientManager._changeListeners should not have obsolete listeners.

- Description for the changelog

• Remove the inactive listeners from AtClientManager._changeListeners list.

[sitaram-kalluri]

Found the root cause of failing end2end_tests. Will fix the issue and push the changes.

[gkc]

As discussed just now, we don't need the 'isActive' flag. The concurrent modification error can be eliminated by having _notifyListeners make a copy of the _changeListeners and iterate through the copy of the list rather than through the list itself.

As a side note - force-pushing your branch is masking useful information about the evolution of the code - without seeing the interim commits, it was completely unclear to me why the isActive flag was added until we had our discussion. Is there a reason you prefer force-push?

[sitaram-kalluri]

As discussed just now, we don't need the 'isActive' flag. The concurrent modification error can be eliminated by having _notifyListeners make a copy of the _changeListeners and iterate through the copy of the list rather than through the list itself.

As a side note - force-pushing your branch is masking useful information about the evolution of the code - without seeing the interim commits, it was completely unclear to me why the isActive flag was added until we had our discussion. Is there a reason you prefer force-push?

@gkc: Sure, will update the code as per our discussion. I generally do a force push to keep the commit history clean. However, I do it only when there are no review comments/discussions on the PR. If there are review comments/discussions on the PR I avoid doing a force push.

[gkc]

@gkc: Sure, will update the code as per our discussion. I generally do a force push to keep the commit history clean. However, I do it only when there are no review comments/discussions on the PR. If there are review comments/discussions on the PR I avoid doing a force push.

I understand. However sometimes (as in this case), squashing the commits makes it impossible to understand how you got to a particular design / implementation decision, without having a conversation.

[sitaram-kalluri]

@gkc: Sure, will update the code as per our discussion. I generally do a force push to keep the commit history clean. However, I do it only when there are no review comments/discussions on the PR. If there are review comments/discussions on the PR I avoid doing a force push.

I understand. However sometimes (as in this case), squashing the commits makes it impossible to understand how you got to a particular design / implementation decision, without having a conversation.

Ok Gary, Will avoid force pushing to the branch.

I've made changes

[gkc]

@VJag Have made some further changes to this branch in 0db8b2b ... please review