Refactored ConnectionManager/Connection.

github · grokys · Nov 8, 2017 · Oct 20, 2017 · Oct 23, 2017 · Oct 23, 2017
commit d98ec51f77e00f0bfc75423e683b3dc72bc1f453
diff --git a/src/GitHub.App/Controllers/UIController.cs b/src/GitHub.App/Controllers/UIController.cs
@@ -219,13 +219,7 @@ public void Start()
                 else // sanity check: it makes zero sense to pass a connection in when calling the auth flow
                     Debug.Assert(false, "Calling the auth flow with a connection makes no sense!");
 
-                connection.Login()
-                    .ObserveOn(RxApp.MainThreadScheduler)
-                    .Subscribe(_ => { }, () =>
-                    {
-                        Debug.WriteLine("Start ({0})", GetHashCode());
-                        Fire(Trigger.Next);
-                    });
+                Fire(Trigger.Next);
             }
             else
             {

diff --git a/src/GitHub.App/Models/RepositoryHosts.cs b/src/GitHub.App/Models/RepositoryHosts.cs
@@ -75,11 +75,6 @@ public RepositoryHosts(
                 (githubLoggedIn, enterpriseLoggedIn) => githubLoggedIn.Value || enterpriseLoggedIn.Value)
                 .ToProperty(this, x => x.IsLoggedInToAnyHost);
 
-            // This part is strictly to support having the IConnectionManager request that a connection
-            // be logged in. It doesn't know about hosts or load anything reactive, so it gets
-            // informed of logins by an observable returned by the event
-            connectionManager.DoLogin += RunLoginHandler;
-
             // monitor the list of connections so we can log out hosts when connections are removed
             disposables.Add(
                 connectionManager.Connections.CreateDerivedCollection(x => x)
@@ -100,24 +95,6 @@ public RepositoryHosts(
             disposables.Add(persistEntepriseHostObs.Subscribe());
         }
 
-        IObservable<IConnection> RunLoginHandler(IConnection connection)
-        {
-            Guard.ArgumentNotNull(connection, nameof(connection));
-
-            var handler = new ReplaySubject<IConnection>();
-            var address = connection.HostAddress;
-            var host = LookupHost(address);
-            if (host == DisconnectedRepositoryHost)
-                LogInFromCache(address)
-                    .Subscribe(c => handler.OnNext(connection), () => handler.OnCompleted());
-            else
-            {
-                handler.OnNext(connection);
-                handler.OnCompleted();
-            }
-            return handler;
-        }
-
         public IRepositoryHost LookupHost(HostAddress address)
         {
             if (address == GitHubHost.Address)
@@ -164,7 +141,7 @@ public IObservable<AuthenticationResult> LogIn(
                             else
                                 enterpriseHost = host;
 
-                            connectionManager.AddConnection(address, usernameOrEmail);
+                            connectionManager.LogIn(address, usernameOrEmail, password);
 
                             if (isDotCom)
                                 this.RaisePropertyChanged(nameof(GitHubHost));
@@ -223,7 +200,7 @@ public IObservable<Unit> LogOut(IRepositoryHost host)
                         GitHubHost = null;
                     else
                         EnterpriseHost = null;
-                    connectionManager.RemoveConnection(address);
+                    connectionManager.LogOut(address);
                     RepositoryHostFactory.Remove(host);
                 });
         }
@@ -237,7 +214,6 @@ protected void Dispose(bool disposing)
 
                 try
                 {
-                    connectionManager.DoLogin -= RunLoginHandler;
                     disposables.Dispose();
                 }
                 catch (Exception e)

diff --git a/src/GitHub.App/SampleData/SampleViewModels.cs b/src/GitHub.App/SampleData/SampleViewModels.cs
@@ -200,23 +200,15 @@ class Conn : IConnection
             public string Username { get; set; }
             public ObservableCollection<ILocalRepositoryModel> Repositories { get; set;  }
 
-            public IObservable<IConnection> Login()
-            {
-                return null;
-            }
+            public Octokit.User User => null;
+            public bool IsLoggedIn => true;
 
-            public void Logout()
-            {
-            }
-
-            public void Dispose()
-            {
-            }
+            public Exception ConnectionError => null;
         }
 
         public RepositoryPublishViewModelDesigner()
         {
-            Connections = new ObservableCollection<IConnection>
+            Connections = new ObservableCollectionEx<IConnection>
             {
                 new Conn() { HostAddress = new HostAddress() },
                 new Conn() { HostAddress = HostAddress.Create("ghe.io") }
@@ -238,7 +230,7 @@ public IReactiveCommand<ProgressState> PublishRepository
             private set;
         }
 
-        public ObservableCollection<IConnection> Connections
+        public IReadOnlyObservableCollection<IConnection> Connections
         {
             get;
             private set;

diff --git a/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs b/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs
@@ -117,7 +117,7 @@ public RepositoryPublishViewModel(
         public bool CanKeepPrivate { get { return canKeepPrivate.Value; } }
 
         public IReactiveCommand<ProgressState> PublishRepository { get; private set; }
-        public ObservableCollection<IConnection> Connections { get; private set; }
+        public IReadOnlyObservableCollection<IConnection> Connections { get; private set; }
 
         IConnection selectedConnection;
         public IConnection SelectedConnection

diff --git a/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs b/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs
@@ -1,6 +1,9 @@
 using System;
 using System.Linq;
 using System.Reactive.Linq;
+using System.Reactive.Threading.Tasks;
+using System.Threading.Tasks;
+using GitHub.Api;
 using GitHub.Models;
 using GitHub.Primitives;
 using GitHub.Services;
@@ -12,33 +15,40 @@ public static class ConnectionManagerExtensions
         public static IObservable<bool> IsLoggedIn(this IConnectionManager cm, IRepositoryHosts hosts)
         {
             Guard.ArgumentNotNull(hosts, nameof(hosts));
-            return cm.Connections.ToObservable()
-                    .SelectMany(c => c.Login())
-                    .Any(c => hosts.LookupHost(c.HostAddress).IsLoggedIn);
+
+            return Observable.FromAsync(async () =>
+            {
+                var connections = await cm.GetLoadedConnections();
+                return connections.Any(x => x.ConnectionError == null);
+            });
         }
 
         public static IObservable<bool> IsLoggedIn(this IConnectionManager cm, IRepositoryHosts hosts, HostAddress address)
         {
             Guard.ArgumentNotNull(hosts, nameof(hosts));
             Guard.ArgumentNotNull(address, nameof(address));
-            return cm.Connections.ToObservable()
-                    .Where(c => c.HostAddress.Equals(address))
-                    .SelectMany(c => c.Login())
-                    .Any(c => hosts.LookupHost(c.HostAddress).IsLoggedIn);
+
+            return Observable.FromAsync(async () =>
+            {
+                var connections = await cm.GetLoadedConnections();
+                return connections.Any(x => x.HostAddress == address && x.ConnectionError == null);
+            });
         }
 
         public static IObservable<bool> IsLoggedIn(this IConnection connection, IRepositoryHosts hosts)
         {
             Guard.ArgumentNotNull(hosts, nameof(hosts));
-            return connection?.Login().Any(c => hosts.LookupHost(c.HostAddress).IsLoggedIn) ?? Observable.Return(false);
+
+            return Observable.Return(connection?.IsLoggedIn ?? false);
         }
 
         public static IObservable<IConnection> GetLoggedInConnections(this IConnectionManager cm, IRepositoryHosts hosts)
         {
             Guard.ArgumentNotNull(hosts, nameof(hosts));
-            return cm.Connections.ToObservable()
-                    .SelectMany(c => c.Login())
-                    .Where(c => hosts.LookupHost(c.HostAddress).IsLoggedIn);
+
+            return cm.GetLoadedConnections()
+                .ToObservable()
+                .Select(x => x.FirstOrDefault(y => y.IsLoggedIn));
         }
 
         public static IObservable<IConnection> LookupConnection(this IConnectionManager cm, ILocalRepositoryModel repository)

diff --git a/src/GitHub.Exports.Reactive/ViewModels/IRepositoryPublishViewModel.cs b/src/GitHub.Exports.Reactive/ViewModels/IRepositoryPublishViewModel.cs
@@ -2,12 +2,13 @@
 using GitHub.Models;
 using ReactiveUI;
 using System.Collections.ObjectModel;
+using GitHub.Extensions;
 
 namespace GitHub.ViewModels
 {
     public interface IRepositoryPublishViewModel : IRepositoryForm
     {
-        ObservableCollection<IConnection> Connections { get; }
+        IReadOnlyObservableCollection<IConnection> Connections { get; }
 
         /// <summary>
         /// Command that creates the repository.

diff --git a/src/GitHub.Exports/Models/IConnection.cs b/src/GitHub.Exports/Models/IConnection.cs
@@ -1,16 +1,41 @@
-using GitHub.Primitives;
-using System;
-using System.Collections.ObjectModel;
-using System.Collections.Specialized;
-using System.Threading.Tasks;
+using System;
+using GitHub.Primitives;
+using Octokit;
 
 namespace GitHub.Models
 {
+    /// <summary>
+    /// Represents a configured connection to a GitHub account.
+    /// </summary>
     public interface IConnection
     {
+        /// <summary>
+        /// Gets the host address of the GitHub instance.
+        /// </summary>
         HostAddress HostAddress { get; }
+
+        /// <summary>
+        /// Gets the username of the GitHub account.
+        /// </summary>
         string Username { get; }
-        IObservable<IConnection> Login();
-        void Logout();
+
+        /// <summary>
+        /// Gets the logged in user.
+        /// </summary>
+        /// <remarks>
+        /// This may be null if <see cref="IsLoggedIn"/> is false.
+        /// </remarks>
+        User User { get; }
+
+        /// <summary>
+        /// Gets a value indicating whether the login of the account succeeded.
+        /// </summary>
+        bool IsLoggedIn { get; }
+
+        /// <summary>
+        /// Gets the exception that occurred when trying to log in, if <see cref="IsLoggedIn"/> is
+        /// false.
+        /// </summary>
+        Exception ConnectionError { get; }
     }
 }
diff --git a/src/GitHub.Exports/Services/Connection.cs b/src/GitHub.Exports/Services/Connection.cs
@@ -4,28 +4,36 @@
 
 namespace GitHub.Services
 {
+    /// <summary>
+    /// Represents a configured connection to a GitHub account.
+    /// </summary>
     public class Connection : IConnection
     {
-        readonly IConnectionManager manager;
-
-        public Connection(IConnectionManager cm, HostAddress hostAddress, string userName)
+        public Connection(
+            HostAddress hostAddress,
+            string userName,
+            Octokit.User user,
+            Exception connectionError)
         {
-            manager = cm;
             HostAddress = hostAddress;
             Username = userName;
+            User = user;
+            ConnectionError = connectionError;
         }
 
-        public HostAddress HostAddress { get; private set; }
-        public string Username { get; private set; }
+        /// <inheritdoc/>
+        public HostAddress HostAddress { get; }
 
-        public IObservable<IConnection> Login()
-        {
-            return manager.RequestLogin(this);
-        }
+        /// <inheritdoc/>
+        public string Username { get; }
 
-        public void Logout()
-        {
-            manager.RequestLogout(this);
-        }
+        /// <inheritdoc/>
+        public Octokit.User User { get; }
+
+        /// <inheritdoc/>
+        public bool IsLoggedIn => ConnectionError == null;
+
+        /// <inheritdoc/>
+        public Exception ConnectionError { get; }
     }
 }
diff --git a/src/GitHub.Exports/Services/IConnectionManager.cs b/src/GitHub.Exports/Services/IConnectionManager.cs
@@ -1,24 +1,65 @@
 using System;
-using System.Collections.ObjectModel;
-using System.Diagnostics.CodeAnalysis;
 using System.Threading.Tasks;
+using GitHub.Extensions;
 using GitHub.Models;
 using GitHub.Primitives;
 
 namespace GitHub.Services
 {
+    /// <summary>
+    /// Manages the configured <see cref="IConnection"/>s to GitHub instances.
+    /// </summary>
     public interface IConnectionManager
     {
-        IConnection CreateConnection(HostAddress address, string username);
-        bool AddConnection(HostAddress address, string username);
-        bool RemoveConnection(HostAddress address);
-        ObservableCollection<IConnection> Connections { get; }
+        /// <summary>
+        /// Gets a collection containing the current connections.
+        /// </summary>
+        /// <remarks>
+        /// This collection is lazily initialized: the first time the <see cref="Connections"/>
+        /// property is accessed, an async load of the configured connections is started. If you
+        /// want to ensure that all connections have been loaded, call
+        /// <see cref="GetLoadedConnections"/>.
+        /// </remarks>
+        IReadOnlyObservableCollection<IConnection> Connections { get; }
 
-        IObservable<IConnection> RequestLogin(IConnection connection);
-        void RequestLogout(IConnection connection);
+        /// <summary>
+        /// Gets a connection with the specified host address.
+        /// </summary>
+        /// <param name="address">The address.</param>
+        /// <returns>
+        /// A task returning the requested connection, or null if the connection was not found.
+        /// </returns>
+        Task<IConnection> GetConnection(HostAddress address);
 
-        // for telling IRepositoryHosts that we need to login from cache
-        [SuppressMessage("Microsoft.Design", "CA1009:DeclareEventHandlersCorrectly")]
-        event Func<IConnection, IObservable<IConnection>> DoLogin;
+        /// <summary>
+        /// Gets the <see cref="Connections"/> collection, after ensuring that it is fully loaded.
+        /// </summary>
+        /// <returns>
+        /// A task returning the fully loaded connections collection.
+        /// </returns>
+        Task<IReadOnlyObservableCollection<IConnection>> GetLoadedConnections();
+
+        /// <summary>
+        /// Attempts to login to a GitHub instance.
+        /// </summary>
+        /// <param name="address">The instance address.</param>
+        /// <param name="username">The username.</param>
+        /// <param name="password">The password.</param>
+        /// <returns>
+        /// A connection if the login succeded. If the login fails, throws an exception. An
+        /// exception is also thrown if an existing connection with the same host address already
+        /// exists.
+        /// </returns>
+        /// <exception cref="InvalidOperationException">
+        /// A connection to the host already exists.
+        /// </exception>
+        Task<IConnection> LogIn(HostAddress address, string username, string password);
+
+        /// <summary>
+        /// Logs out of a GitHub instance.
+        /// </summary>
+        /// <param name="address"></param>
+        /// <returns>A task tracking the operation.</returns>
+        Task LogOut(HostAddress address);
     }
-}
+}
diff --git a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs
@@ -383,7 +383,7 @@ public void DoCreate()
 
         public void SignOut()
         {
-            SectionConnection.Logout();
+            connectionManager.LogOut(SectionConnection.HostAddress);
         }
 
         public void Login()