From 59f7ea318099b48312589dbbf799e09b6775e83d Mon Sep 17 00:00:00 2001
From: Romain Bertozzi <romain.bertozzi@savoirfairelinux.com>
Date: Mon, 3 Apr 2017 22:38:14 -0400
Subject: [PATCH] arch: remove singleton and add DI
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch removes the singleton of the AccountService and
AccountAdapter.

These elements are still instantiated and kept once in the application
thanks to the AppDelegate.
They are then injected by initializers to the rest of the app.

In order to achieve this, we had to change the delegate connection
between the AccountAdapter and its Service. Indeed, we do not have a
singleton pattern anymore and we can not keep a "self" reference
because of the exportable_callback behaviour.
So, we set the delegate to a static delegate.

Tuleap: #1391
Change-Id: I56d6e79f7d3c09d6ac3166643fa293cf6df67555
Reviewed-by: Silbino Gonçalves Matado <silbino.gmatado@savoirfairelinux.com>
---
 .../Account/CreateRingAccountViewModel.swift  | 16 +++++++++---
 Ring/Ring/AppDelegate.swift                   |  7 ++---
 Ring/Ring/Bridging/AccountAdapter.h           |  4 +--
 Ring/Ring/Bridging/AccountAdapter.mm          | 26 +++++++++++--------
 .../MainTabBar/MainTabBarViewController.swift |  4 ++-
 Ring/Ring/MeViewController.swift              |  2 +-
 Ring/Ring/Services/AccountsService.swift      | 23 +++++++---------
 .../CreateRingAccountViewController.swift     |  2 +-
 8 files changed, 47 insertions(+), 37 deletions(-)

diff --git a/Ring/Ring/Account/CreateRingAccountViewModel.swift b/Ring/Ring/Account/CreateRingAccountViewModel.swift
index 194e4ebf1..aa17e10ca 100644
--- a/Ring/Ring/Account/CreateRingAccountViewModel.swift
+++ b/Ring/Ring/Account/CreateRingAccountViewModel.swift
@@ -43,18 +43,26 @@ class CreateRingAccountViewModel {
      */
     fileprivate var account: AccountModel?
 
+    /**
+     The accountService instance injected in initializer.
+     */
+    fileprivate let accountService: AccountsService
+
     /**
      Default constructor
      */
-    init() {
+    init(withAccountService accountService: AccountsService) {
         self.account = nil
+        self.accountService = accountService
     }
 
     /**
      Constructor with AccountModel.
      */
-    init(withAccountModel account: AccountModel?) {
+    init(withAccountService accountService: AccountsService,
+         accountModel account: AccountModel?) {
         self.account = account
+        self.accountService = accountService
     }
 
     /**
@@ -81,7 +89,7 @@ class CreateRingAccountViewModel {
                     //~ simultaneously authorized.
                     self?.addAccountDisposable?.dispose()
                     //~ Subscribe on the AccountsService responseStream to get results.
-                    self?.addAccountDisposable = AccountsService.sharedInstance
+                    self?.addAccountDisposable = self?.accountService
                         .sharedResponseStream
                         .subscribe(onNext:{ (event) in
                             if event.eventType == ServiceEventType.AccountsChanged {
@@ -93,7 +101,7 @@ class CreateRingAccountViewModel {
                     self?.addAccountDisposable?.addDisposableTo((self?.disposeBag)!)
 
                     //~ Launch the action.
-                    AccountsService.sharedInstance.addAccount()
+                    self?.accountService.addAccount()
                 },
                 onError: { (error) in
                     onErrorCallback?(error)
diff --git a/Ring/Ring/AppDelegate.swift b/Ring/Ring/AppDelegate.swift
index 709f31b11..7c4c8673f 100644
--- a/Ring/Ring/AppDelegate.swift
+++ b/Ring/Ring/AppDelegate.swift
@@ -26,7 +26,8 @@ import CoreData
 class AppDelegate: UIResponder, UIApplicationDelegate {
 
     var window: UIWindow?
-    let daemonService = DaemonService(dRingAdaptor: DRingAdapter())
+    static let daemonService = DaemonService(dRingAdaptor: DRingAdapter())
+    static let accountService = AccountsService(withAccountAdapter: AccountAdapter())
 
     func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplicationLaunchOptionsKey: Any]?) -> Bool {
         SystemAdapter().registerConfigurationHandler()
@@ -123,7 +124,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
     // MARK: - Ring Daemon
     fileprivate func startDaemon() {
         do {
-            try self.daemonService.startDaemon()
+            try AppDelegate.daemonService.startDaemon()
         } catch StartDaemonError.InitializationFailure {
             print("Daemon failed to initialize.")
         } catch StartDaemonError.StartFailure {
@@ -137,7 +138,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
 
     fileprivate func stopDaemon() {
         do {
-            try self.daemonService.stopDaemon()
+            try AppDelegate.daemonService.stopDaemon()
         } catch StopDaemonError.DaemonNotRunning {
             print("Daemon failed to stop because it was not already running.")
         } catch {
diff --git a/Ring/Ring/Bridging/AccountAdapter.h b/Ring/Ring/Bridging/AccountAdapter.h
index 8bb44cda0..4cced56fc 100644
--- a/Ring/Ring/Bridging/AccountAdapter.h
+++ b/Ring/Ring/Bridging/AccountAdapter.h
@@ -41,9 +41,7 @@
 /**
  Delegate where all the accounts events will be forwarded.
  */
-@property (nonatomic, weak) id <AccountAdapterDelegate> delegate;
-
-+ (instancetype)sharedManager;
+@property (class, nonatomic, weak) id <AccountAdapterDelegate> delegate;
 
 - (NSDictionary *)getAccountDetails:(NSString *)accountID;
 
diff --git a/Ring/Ring/Bridging/AccountAdapter.mm b/Ring/Ring/Bridging/AccountAdapter.mm
index 996635b94..d13441112 100644
--- a/Ring/Ring/Bridging/AccountAdapter.mm
+++ b/Ring/Ring/Bridging/AccountAdapter.mm
@@ -30,16 +30,10 @@
 
 using namespace DRing;
 
-#pragma mark Singleton Methods
-+ (instancetype)sharedManager {
-    static AccountAdapter* sharedMyManager = nil;
-    static dispatch_once_t onceToken;
-    dispatch_once(&onceToken, ^{
-        sharedMyManager = [[self alloc] init];
-    });
-    return sharedMyManager;
-}
+/// Static delegate that will receive the propagated daemon events
+static id <AccountAdapterDelegate> _delegate;
 
+#pragma mark Init
 - (id)init {
     if (self = [super init]) {
         [self registerConfigurationHandler];
@@ -53,8 +47,8 @@ using namespace DRing;
     std::map<std::string, std::shared_ptr<CallbackWrapperBase>> confHandlers;
     confHandlers.insert(exportable_callback<ConfigurationSignal::AccountsChanged>([&]() {
         //~ Using sharedManager to avoid as possible to retain self in the block.
-        if ([[AccountAdapter sharedManager] delegate]) {
-            [[[AccountAdapter sharedManager] delegate] accountsChanged];
+        if (AccountAdapter.delegate) {
+            [AccountAdapter.delegate accountsChanged];
         }
     }));
     registerConfHandlers(confHandlers);
@@ -102,4 +96,14 @@ using namespace DRing;
 }
 #pragma mark -
 
+#pragma mark AccountAdapterDelegate
++ (id <AccountAdapterDelegate>)delegate {
+    return _delegate;
+}
+
++ (void) setDelegate:(id<AccountAdapterDelegate>)delegate {
+    _delegate = delegate;
+}
+#pragma mark -
+
 @end
diff --git a/Ring/Ring/MainTabBar/MainTabBarViewController.swift b/Ring/Ring/MainTabBar/MainTabBarViewController.swift
index 5e4fe1494..678d30a0b 100644
--- a/Ring/Ring/MainTabBar/MainTabBarViewController.swift
+++ b/Ring/Ring/MainTabBar/MainTabBarViewController.swift
@@ -21,9 +21,11 @@
 import UIKit
 
 class MainTabBarViewController: UITabBarController {
+    fileprivate let accountService = AppDelegate.accountService
+
     override func viewDidAppear(_ animated: Bool) {
         super.viewDidAppear(animated)
-        if !AccountsService.sharedInstance.hasAccounts() {
+        if !accountService.hasAccounts() {
             self.presentWalkthrough()
         }
     }
diff --git a/Ring/Ring/MeViewController.swift b/Ring/Ring/MeViewController.swift
index 5fe598349..44cadd878 100644
--- a/Ring/Ring/MeViewController.swift
+++ b/Ring/Ring/MeViewController.swift
@@ -23,7 +23,7 @@ import UIKit
 class MeViewController: UIViewController, UITableViewDelegate, UITableViewDataSource {
 
     // MARK: - Properties
-    let accountService = AccountsService.sharedInstance
+    let accountService = AppDelegate.accountService
     @IBOutlet weak var accountTableView: UITableView!
     @IBOutlet weak var nameLabel: UILabel!
     @IBOutlet weak var qrImageView: UIImageView!
diff --git a/Ring/Ring/Services/AccountsService.swift b/Ring/Ring/Services/AccountsService.swift
index 23220664b..3dba6b6af 100644
--- a/Ring/Ring/Services/AccountsService.swift
+++ b/Ring/Ring/Services/AccountsService.swift
@@ -25,10 +25,9 @@ import RxSwift
 class AccountsService: AccountAdapterDelegate {
     // MARK: Private members
     /**
-     AccountConfigurationManagerAdaptator instance.
-     Used to register the service to daemon events.
+     Used to register the service to daemon events, injected by constructor.
      */
-    fileprivate let confAdapter = AccountAdapter.sharedManager() as AccountAdapter
+    fileprivate let accountAdapter: AccountAdapter
 
     /**
      Fileprivate Accounts list.
@@ -74,10 +73,7 @@ class AccountsService: AccountAdapterDelegate {
      */
     var sharedResponseStream: Observable<ServiceEvent>
 
-    // MARK: - Singleton
-    static let sharedInstance = AccountsService()
-
-    fileprivate init() {
+    init(withAccountAdapter accountAdapter: AccountAdapter) {
         self.accountList = []
 
         self.responseStream.addDisposableTo(disposeBag)
@@ -85,9 +81,10 @@ class AccountsService: AccountAdapterDelegate {
         //~ Create a shared stream based on the responseStream one.
         self.sharedResponseStream = responseStream.share()
 
-        //~ Registering to the AccountConfigurationManagerAdaptator with self as delegate in order
-        //~ to receive delegation callbacks.
-        self.confAdapter.delegate = self
+        self.accountAdapter = accountAdapter
+        //~ Registering to the accountAdatpter with self as delegate in order to receive delegation
+        //~ callbacks.
+        AccountAdapter.delegate = self
     }
 
     // MARK: - Methods
@@ -105,7 +102,7 @@ class AccountsService: AccountAdapterDelegate {
 
     func addAccount() {
         // TODO: This need work for all account type
-        let details:NSMutableDictionary? = confAdapter.getAccountTemplate("RING")
+        let details:NSMutableDictionary? = self.accountAdapter.getAccountTemplate("RING")
         if details == nil {
             print("Error retrieving Ring account template, can not continue");
             return;
@@ -113,13 +110,13 @@ class AccountsService: AccountAdapterDelegate {
         details!.setValue("iOS", forKey: "Account.alias")
         details!.setValue("iOS", forKey: "Account.displayName")
         let convertedDetails = details as NSDictionary? as? [AnyHashable: Any] ?? [:]
-        let addResult:String! = confAdapter.addAccount(convertedDetails)
+        let addResult:String! = self.accountAdapter.addAccount(convertedDetails)
         print(addResult);
     }
 
     func removeAccount(_ row: Int) {
         if row < accountList.count {
-            confAdapter.removeAccount(accountList[row].id)
+            self.accountAdapter.removeAccount(accountList[row].id)
         }
     }
 
diff --git a/Ring/Ring/Walkthrough/CreateRingAccountViewController.swift b/Ring/Ring/Walkthrough/CreateRingAccountViewController.swift
index 94c474677..8f922b3b7 100644
--- a/Ring/Ring/Walkthrough/CreateRingAccountViewController.swift
+++ b/Ring/Ring/Walkthrough/CreateRingAccountViewController.swift
@@ -25,7 +25,7 @@ import RxSwift
 
 class CreateRingAccountViewController: UIViewController {
 
-    var mAccountViewModel = CreateRingAccountViewModel()
+    var mAccountViewModel = CreateRingAccountViewModel(withAccountService: AppDelegate.accountService)
 
     @IBOutlet weak var mCreateAccountButton: RoundedButton!
 
-- 
GitLab