Refactoring ConnectionManager/RepositoryHost(s)

src/GitHub.App/Controllers/UIController.cs

@@ -14,6 +14,7 @@
 using System.Reactive.Disposables;
 using System.Reactive.Linq;
 using System.Reactive.Subjects;
+using System.Reactive.Threading.Tasks;
 using System.Windows;
 using GitHub.Logging;
 
@@ -220,7 +221,7 @@ public void Start()
                 else // sanity check: it makes zero sense to pass a connection in when calling the auth flow
                     Log.Assert(false, "Calling the auth flow with a connection makes no sense!");
 
-                connection.Login()
+                hosts.EnsureInitialized().ToObservable()
                     .ObserveOn(RxApp.MainThreadScheduler)
                     .Subscribe(_ => { }, () =>
                     {
@@ -445,8 +446,7 @@ void ConfigureSingleViewLogic(UIControllerFlow flow, UIViewType type)
 
         UIControllerFlow SelectActiveFlow()
         {
-            var host = connection != null ? hosts.LookupHost(connection.HostAddress) : null;
-            var loggedIn = host?.IsLoggedIn ?? false;
+            var loggedIn = connection?.IsLoggedIn ?? false;
             return loggedIn ? selectedFlow : UIControllerFlow.Authentication;
         }
 
@@ -711,7 +711,7 @@ public void Dispose()
         public bool IsStopped => uiStateMachine.IsInState(UIViewType.None) || stopping;
         public UIControllerFlow CurrentFlow => activeFlow;
         public UIControllerFlow SelectedFlow => selectedFlow;
-        bool LoggedIn => connection != null && hosts.LookupHost(connection.HostAddress).IsLoggedIn;
+        bool LoggedIn => connection?.IsLoggedIn ?? false;
         bool? Success { get; set; }
     }
 }

src/GitHub.App/Factories/RepositoryHostFactory.cs

@@ -1,12 +1,7 @@
-using System;
-using System.ComponentModel.Composition;
-using GitHub.Caches;
+using System.ComponentModel.Composition;
+using System.Threading.Tasks;
 using GitHub.Models;
-using GitHub.Primitives;
 using GitHub.Services;
-using System.Reactive.Disposables;
-using System.Threading.Tasks;
-using GitHub.Api;
 
 namespace GitHub.Factories
 {
@@ -16,59 +11,27 @@ public class RepositoryHostFactory : IRepositoryHostFactory
     {
         readonly IApiClientFactory apiClientFactory;
         readonly IHostCacheFactory hostCacheFactory;
-        readonly ILoginManager loginManager;
-        readonly IKeychain keychain;
         readonly IAvatarProvider avatarProvider;
-        readonly CompositeDisposable hosts = new CompositeDisposable();
-        readonly IUsageTracker usage;
 
         [ImportingConstructor]
         public RepositoryHostFactory(
             IApiClientFactory apiClientFactory,
             IHostCacheFactory hostCacheFactory,
-            ILoginManager loginManager,
-            IKeychain keychain,
-            IAvatarProvider avatarProvider,
-            IUsageTracker usage)
+            IAvatarProvider avatarProvider)
         {
             this.apiClientFactory = apiClientFactory;
             this.hostCacheFactory = hostCacheFactory;
-            this.loginManager = loginManager;
-            this.keychain = keychain;
             this.avatarProvider = avatarProvider;
-            this.usage = usage;
         }
 
-        public async Task<IRepositoryHost> Create(HostAddress hostAddress)
+        public async Task<IRepositoryHost> Create(IConnection connection)
         {
+            var hostAddress = connection.HostAddress;
             var apiClient = await apiClientFactory.Create(hostAddress);
             var hostCache = await hostCacheFactory.Create(hostAddress);
             var modelService = new ModelService(apiClient, hostCache, avatarProvider);
-            var host = new RepositoryHost(apiClient, modelService, loginManager, keychain, usage);
-            hosts.Add(host);
+            var host = new RepositoryHost(connection, apiClient, modelService);
             return host;
         }
-
-        public void Remove(IRepositoryHost host)
-        {
-            hosts.Remove(host);
-        }
-
-        bool disposed;
-        protected virtual void Dispose(bool disposing)
-        {
-            if (disposing)
-            {
-                if (disposed) return;
-                disposed = true;
-                hosts.Dispose();
-            }
-        }
-
-        public void Dispose()
-        {
-            Dispose(true);
-            GC.SuppressFinalize(this);
-        }
     }
 }

src/GitHub.App/Models/DisconnectedRepositoryHosts.cs

@@ -48,10 +48,5 @@ public IObservable<Unit> LogOut()
         {
             return Observable.Return(Unit.Default);
         }
-
-        public void Dispose()
-        {
-            GC.SuppressFinalize(this);
-        }
     }
 }

src/GitHub.App/Models/RepositoryHost.cs

@@ -1,22 +1,13 @@
 using System;
 using System.Diagnostics;
-using System.Diagnostics.CodeAnalysis;
-using System.Globalization;
-using System.Linq;
-using System.Reactive;
-using System.Reactive.Linq;
-using System.Reactive.Threading.Tasks;
-using System.Threading.Tasks;
 using GitHub.Api;
-using GitHub.Authentication;
-using GitHub.Caches;
 using GitHub.Extensions;
 using GitHub.Logging;
 using GitHub.Primitives;
 using GitHub.Services;
-using Octokit;
 using ReactiveUI;
 using Serilog;
+using static System.FormattableString;
 
 namespace GitHub.Models
 {
@@ -25,145 +16,28 @@ public class RepositoryHost : ReactiveObject, IRepositoryHost
     {
         static readonly ILogger log = LogManager.ForContext<RepositoryHosts>();
 
-        readonly ILoginManager loginManager;
-        readonly HostAddress hostAddress;
-        readonly IKeychain keychain;
-        readonly IUsageTracker usage;
-
-        bool isLoggedIn;
+        readonly IConnection connection;
 
         public RepositoryHost(
+            IConnection connection,
             IApiClient apiClient,
-            IModelService modelService,
-            ILoginManager loginManager,
-            IKeychain keychain,
-            IUsageTracker usage)
+            IModelService modelService)
         {
+            Guard.ArgumentNotNull(connection, nameof(connection));
+            Guard.ArgumentNotNull(apiClient, nameof(apiClient));
+            Guard.ArgumentNotNull(modelService, nameof(modelService));
+
+            this.connection = connection;
             ApiClient = apiClient;
             ModelService = modelService;
-            this.loginManager = loginManager;
-            this.keychain = keychain;
-            this.usage = usage;
-
-            log.Assert(apiClient.HostAddress != null, "HostAddress of an api client shouldn't be null");
-            Address = apiClient.HostAddress;
-            hostAddress = apiClient.HostAddress;
-            Title = apiClient.HostAddress.Title;
-        }
-
-        public HostAddress Address { get; private set; }
-
-        public IApiClient ApiClient { get; private set; }
-
-        public bool IsLoggedIn
-        {
-            get { return isLoggedIn; }
-            private set { this.RaiseAndSetIfChanged(ref isLoggedIn, value); }
-        }
-
-        public string Title { get; private set; }
-
-        [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")]
-        public IObservable<AuthenticationResult> LogInFromCache()
-        {
-            Func<Task<AuthenticationResult>> f = async () =>
-            {
-                try
-                {
-                    var user = await loginManager.LoginFromCache(Address, ApiClient.GitHubClient);
-                    var accountCacheItem = new AccountCacheItem(user);
-                    usage.IncrementLoginCount().Forget();
-                    await ModelService.InsertUser(accountCacheItem);
-
-                    if (user != null)
-                    {
-                        IsLoggedIn = true;
-                        return AuthenticationResult.Success;
-                    }
-                    else
-                    {
-                        return AuthenticationResult.VerificationFailure;
-                    }
-                }
-                catch (AuthorizationException)
-                {
-                    return AuthenticationResult.CredentialFailure;
-                }
-            };
-
-            return f().ToObservable();
-        }
-
-        public IObservable<AuthenticationResult> LogIn(string usernameOrEmail, string password)
-        {
-            Guard.ArgumentNotEmptyString(usernameOrEmail, nameof(usernameOrEmail));
-            Guard.ArgumentNotEmptyString(password, nameof(password));
-
-            return Observable.Defer(async () =>
-            {
-                var user = await loginManager.Login(Address, ApiClient.GitHubClient, usernameOrEmail, password);
-                var accountCacheItem = new AccountCacheItem(user);
-                usage.IncrementLoginCount().Forget();
-                await ModelService.InsertUser(accountCacheItem);
-
-                if (user != null)
-                {
-                    IsLoggedIn = true;
-                    return Observable.Return(AuthenticationResult.Success);
-                }
-                else
-                {
-                    return Observable.Return(AuthenticationResult.VerificationFailure);
-                }
-            });
-        }
-
-        public IObservable<Unit> LogOut()
-        {
-            if (!IsLoggedIn) return Observable.Return(Unit.Default);
-
-            log.Information("Logged off of host '{ApiUri}'", hostAddress.ApiUri);
-
-            return keychain.Delete(Address).ToObservable()
-                .Catch<Unit, Exception>(e =>
-                {
-                    log.Warning(e, "ASSERT! Failed to erase login. Going to invalidate cache anyways");
-                    return Observable.Return(Unit.Default);
-                })
-                .SelectMany(_ => ModelService.InvalidateAll())
-                .Catch<Unit, Exception>(e =>
-                {
-                    log.Warning(e, "ASSERT! Failed to invaldiate caches");
-                    return Observable.Return(Unit.Default);
-                })
-                .ObserveOn(RxApp.MainThreadScheduler)
-                .Finally(() =>
-                {
-                    IsLoggedIn = false;
-                });
         }
 
-        protected virtual void Dispose(bool disposing)
-        {}
+        public HostAddress Address => ApiClient.HostAddress;
+        public IApiClient ApiClient { get;}
+        public bool IsLoggedIn => connection.IsLoggedIn;
+        public IModelService ModelService { get; }
+        public string Title => ApiClient.HostAddress.Title;
 
-        public void Dispose()
-        {
-            Dispose(true);
-            GC.SuppressFinalize(this);
-        }
-
-        internal string DebuggerDisplay
-        {
-            get
-            {
-                return string.Format(CultureInfo.InvariantCulture, "RepositoryHost: {0} {1}", Title, hostAddress.ApiUri);
-            }
-        }
-
-        public IModelService ModelService
-        {
-            get;
-            private set;
-        }
+        internal string DebuggerDisplay => Invariant($"RepositoryHost: {Title} {Address.ApiUri}");
     }
 }