Merge pull request #189 from github/shana/fix-iconnection-when-logged-in

haacked · haacked · commit 972ebd53ac97 · 2016-01-13T10:26:48.000-08:00
Fix connections not being passed on to view models
diff --git a/src/GitHub.App/Controllers/UIController.cs b/src/GitHub.App/Controllers/UIController.cs
@@ -18,6 +18,7 @@
 using ReactiveUI;
 using Stateless;
 using System.Collections.Specialized;
+using System.Linq;
 
 namespace GitHub.Controllers
 {
@@ -227,7 +228,11 @@ public void Start([AllowNull] IConnection connection)
         {
             if (connection != null)
             {
-                uiProvider.AddService(typeof(IConnection), connection);
+                if (currentFlow != UIControllerFlow.Authentication)
+                    uiProvider.AddService(connection);
+                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()
                     .Select(c => hosts.LookupHost(connection.HostAddress))
                     .Do(host =>
@@ -254,13 +259,21 @@ public void Start([AllowNull] IConnection connection)
                     .IsLoggedIn(hosts)
                     .Do(loggedin =>
                     {
-                        if (!loggedin && currentFlow != UIControllerFlow.Authentication)
+                        if (currentFlow != UIControllerFlow.Authentication)
                         {
-                            connectionAdded = (s, e) => {
-                                if (e.Action == NotifyCollectionChangedAction.Add)
-                                    uiProvider.AddService(typeof(IConnection), e.NewItems[0]);
-                            };
-                            connectionManager.Connections.CollectionChanged += connectionAdded;
+                            if (loggedin) // register the first available connection so the viewmodel can use it
+                                uiProvider.AddService(connectionManager.Connections.First(c => hosts.LookupHost(c.HostAddress).IsLoggedIn));
+                            else
+                            {
+                                // a connection will be added to the list when auth is done, register it so the next
+                                // viewmodel can use it
+                                connectionAdded = (s, e) =>
+                                {
+                                    if (e.Action == NotifyCollectionChangedAction.Add)
+                                        uiProvider.AddService(typeof(IConnection), e.NewItems[0]);
+                                };
+                                connectionManager.Connections.CollectionChanged += connectionAdded;
+                            }
                         }
 
                         machine.Configure(UIViewType.None)
diff --git a/src/GitHub.App/Models/ConnectionRepositoryHostMap.cs b/src/GitHub.App/Models/ConnectionRepositoryHostMap.cs
@@ -10,7 +10,7 @@ public class ConnectionRepositoryHostMap : IConnectionRepositoryHostMap
     {
         [ImportingConstructor]
         public ConnectionRepositoryHostMap(IUIProvider provider, IRepositoryHosts hosts)
-            : this(provider.GetService<IConnection>(), hosts)
+            : this(provider.TryGetService<IConnection>(), hosts)
         {
         }
 
diff --git a/src/GitHub.Exports/Extensions/ServiceProviderExtensions.cs b/src/GitHub.Exports/Extensions/ServiceProviderExtensions.cs
@@ -15,50 +15,35 @@ public static class IServiceProviderExtensions
     {
         static IUIProvider cachedUIProvider = null;
 
-        public static T TryGetService<T>(this IServiceProvider serviceProvider) where T : class
-        {
-            return serviceProvider.TryGetService(typeof(T)) as T;
-        }
-
         public static object TryGetService(this IServiceProvider serviceProvider, Type type)
         {
             if (cachedUIProvider != null && type == typeof(IUIProvider))
                 return cachedUIProvider;
 
             var ui = serviceProvider as IUIProvider;
-            if (ui != null)
-                return ui.TryGetService(type);
-            else
-            {
-                try
-                {
-                    return GetServiceAndCache(serviceProvider, type, ref cachedUIProvider);
-                }
-                catch (Exception ex)
-                {
-                    Debug.Print(ex.ToString());
-                }
-                return null;
-            }
+            return ui != null
+                ? ui.TryGetService(type)
+                : GetServiceAndCache(serviceProvider, type, ref cachedUIProvider);
         }
-
-        public static T GetService<T>(this IServiceProvider serviceProvider)
+        public static T GetExportedValue<T>(this IServiceProvider serviceProvider) where T : class
         {
             if (cachedUIProvider != null && typeof(T) == typeof(IUIProvider))
                 return (T)cachedUIProvider;
 
-            return (T)GetServiceAndCache(serviceProvider, typeof(T), ref cachedUIProvider);
+            var ui = serviceProvider as IUIProvider;
+            return ui != null
+                ? ui.TryGetService(typeof(T)) as T
+                : GetExportedValueAndCache<T, IUIProvider>(ref cachedUIProvider);
         }
 
-        public static T GetExportedValue<T>(this IServiceProvider serviceProvider)
+        public static T TryGetService<T>(this IServiceProvider serviceProvider) where T : class
         {
-            if (cachedUIProvider != null && typeof(T) == typeof(IUIProvider))
-                return (T)cachedUIProvider;
+            return serviceProvider.TryGetService(typeof(T)) as T;
+        }
 
-            var ui = serviceProvider as IUIProvider;
-            return ui != null
-                ? ui.GetService<T>()
-                : GetExportedValueAndCache<T, IUIProvider>(ref cachedUIProvider);
+        public static T GetService<T>(this IServiceProvider serviceProvider) where T : class
+        {
+            return serviceProvider.TryGetService(typeof(T)) as T;
         }
 
         public static ITeamExplorerSection GetSection(this IServiceProvider serviceProvider, Guid section)
@@ -68,18 +53,27 @@ public static ITeamExplorerSection GetSection(this IServiceProvider serviceProvi
 
         static object GetServiceAndCache<CacheType>(IServiceProvider provider, Type type, ref CacheType cache)
         {
-            var ret = provider.GetService(type);
-            if (type == typeof(CacheType))
+            object ret = null;
+            try
+            {
+                ret = provider.GetService(type);
+            }
+            catch (Exception ex)
+            {
+                Debug.Print(ex.ToString());
+                VisualStudio.VsOutputLogger.WriteLine("GetServiceAndCache: Could not obtain instance of '{0}'", type);
+            }
+            if (ret != null && type == typeof(CacheType))
                 cache = (CacheType)ret;
             return ret;
         }
 
-        static T GetExportedValueAndCache<T, CacheType>(ref CacheType cache)
+        static T GetExportedValueAndCache<T, CacheType>(ref CacheType cache) where T : class
         {
-            var ret = VisualStudio.Services.ComponentModel.DefaultExportProvider.GetExportedValue<T>();
-            if (typeof(T) == typeof(CacheType))
-                cache = (CacheType)(object)ret;
-            return ret;
+            object ret = VisualStudio.Services.ComponentModel.DefaultExportProvider.GetExportedValueOrDefault<T>();
+            if (ret != null && typeof(T) == typeof(CacheType))
+                cache = (CacheType)ret;
+            return ret as T;
         }
 
         public static void AddTopLevelMenuItem(this IServiceProvider provider,
diff --git a/src/GitHub.Exports/Services/IUIProvider.cs b/src/GitHub.Exports/Services/IUIProvider.cs
@@ -18,6 +18,7 @@ public interface IUIProvider : IServiceProvider
         T TryGetService<T>() where T : class;
 
         void AddService(Type t, object instance);
+        void AddService<T>(T instance);
         void RemoveService(Type t);
 
         IObservable<UserControl> SetupUI(UIControllerFlow controllerFlow, IConnection connection);
diff --git a/src/GitHub.VisualStudio/Services/UIProvider.cs b/src/GitHub.VisualStudio/Services/UIProvider.cs
@@ -144,6 +144,11 @@ public Ret GetService<T, Ret>() where Ret : class
             return GetService<T>() as Ret;
         }
 
+        public void AddService<T>(T instance)
+        {
+            AddService(typeof(T), instance);
+        }
+
         public void AddService(Type t, object instance)
         {
             if (!Initialized)
diff --git a/src/UnitTests/GitHub.App/Controllers/UIControllerTests.cs b/src/UnitTests/GitHub.App/Controllers/UIControllerTests.cs
@@ -13,6 +13,7 @@
 using ReactiveUI;
 using System.Collections.Generic;
 using GitHub.Authentication;
+using System.Collections.ObjectModel;
 
 public class UIControllerTests
 {
@@ -55,8 +56,7 @@ public void ShowingCloneDialogWithoutBeingLoggedInShowsLoginDialog()
             var loginView = factory.GetView(GitHub.Exports.UIViewType.Login);
             loginView.Value.Cancel.Returns(Observable.Empty<object>());
             var cm = provider.GetConnectionManager();
-            var cons = new System.Collections.ObjectModel.ObservableCollection<IConnection>();
-            cm.Connections.Returns(cons);
+            cm.Connections.Returns(new ObservableCollection<IConnection>());
 
             using (var uiController = new UIController((IUIProvider)provider, hosts, factory, cm, LazySubstitute.For<ITwoFactorChallengeHandler>()))
             {
@@ -82,6 +82,7 @@ public void ShowingCloneDialogWhenLoggedInShowsCloneDialog()
             var connection = provider.GetConnection();
             connection.Login().Returns(Observable.Return(connection));
             var cm = provider.GetConnectionManager();
+            cm.Connections.Returns(new ObservableCollection<IConnection> { connection });
             var host = hosts.GitHubHost;
             hosts.LookupHost(connection.HostAddress).Returns(host);
             host.IsLoggedIn.Returns(true);
@@ -99,5 +100,34 @@ public void ShowingCloneDialogWhenLoggedInShowsCloneDialog()
                 uiController.Start(connection);
             }
         }
+
+        [STAFact]
+        public void CloneDialogLoggedInWithoutConnection()
+        {
+            var provider = Substitutes.GetFullyMockedServiceProvider();
+            var hosts = provider.GetRepositoryHosts();
+            var factory = SetupFactory(provider);
+            var connection = provider.GetConnection();
+            connection.Login().Returns(Observable.Return(connection));
+            var cm = provider.GetConnectionManager();
+            cm.Connections.Returns(new ObservableCollection<IConnection> { connection });
+            var host = hosts.GitHubHost;
+            hosts.LookupHost(connection.HostAddress).Returns(host);
+            host.IsLoggedIn.Returns(true);
+
+            using (var uiController = new UIController((IUIProvider)provider, hosts, factory, cm, LazySubstitute.For<ITwoFactorChallengeHandler>()))
+            {
+                var list = new List<IView>();
+                uiController.SelectFlow(UIControllerFlow.Clone)
+                    .Subscribe(uc => list.Add(uc as IView),
+                                () =>
+                                {
+                                    Assert.Equal(1, list.Count);
+                                    Assert.IsAssignableFrom<IViewFor<IRepositoryCloneViewModel>>(list[0]);
+                                    ((IUIProvider)provider).Received().AddService(connection);
+                                });
+                uiController.Start(null);
+            }
+        }
     }
 }
diff --git a/src/UnitTests/GitHub.Exports/VSServicesTests.cs b/src/UnitTests/GitHub.Exports/VSServicesTests.cs
@@ -16,6 +16,7 @@ public void CallsCloneOnVsProvidedCloneService(bool recurseSubmodules, CloneOpti
             var provider = Substitute.For<IUIProvider>();
             var gitRepositoriesExt = Substitute.For<IGitRepositoriesExt>();
             provider.GetService(typeof(IGitRepositoriesExt)).Returns(gitRepositoriesExt);
+            provider.TryGetService(typeof(IGitRepositoriesExt)).Returns(gitRepositoriesExt);
             var vsServices = new VSServices(provider);
 
             vsServices.Clone("https://github.com/github/visualstudio", @"c:\fake\ghfvs", recurseSubmodules);

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@ public class ConnectionRepositoryHostMap : IConnectionRepositoryHostMap`
`10`	`10`	`{`
`11`	`11`	`[ImportingConstructor]`
`12`	`12`	`public ConnectionRepositoryHostMap(IUIProvider provider, IRepositoryHosts hosts)`
`13`		`- : this(provider.GetService<IConnection>(), hosts)`
	`13`	`+ : this(provider.TryGetService<IConnection>(), hosts)`
`14`	`14`	`{`
`15`	`15`	`}`
`16`	`16`
Original file line number	Diff line number	Diff line change
`@@ -144,6 +144,11 @@ public Ret GetService<T, Ret>() where Ret : class`
`144`	`144`	`return GetService<T>() as Ret;`
`145`	`145`	`}`
`146`	`146`
	`147`	`+ public void AddService<T>(T instance)`
	`148`	`+ {`
	`149`	`+ AddService(typeof(T), instance);`
	`150`	`+ }`
	`151`	`+`
`147`	`152`	`public void AddService(Type t, object instance)`
`148`	`153`	`{`
`149`	`154`	`if (!Initialized)`