Making TeamExplorerServiceHolder testable

src/GitHub.Exports/GitHub.Exports.csproj

@@ -265,6 +265,8 @@
     <Compile Include="Primitives\HostAddress.cs" />
     <Compile Include="UI\IUIController.cs" />
     <Compile Include="Models\IPullRequestModel.cs" />
+    <Compile Include="Services\IVSGitExt.cs" />
+    <Compile Include="Services\IVSUIContext.cs" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\..\submodules\octokit.net\Octokit\Octokit.csproj">

src/GitHub.Exports/Services/IVSGitExt.cs

@@ -0,0 +1,15 @@
+using GitHub.Models;
+using System;
+using System.Collections.Generic;
+using System.ComponentModel;
+using System.ComponentModel.Composition;
+
+namespace GitHub.Services
+{
+    public interface IVSGitExt
+    {
+        void Refresh(IServiceProvider serviceProvider);
+        IEnumerable<ILocalRepositoryModel> ActiveRepositories { get; }
+        event Action ActiveRepositoriesChanged;
+    }
+}

src/GitHub.Exports/Services/IVSUIContext.cs

@@ -0,0 +1,21 @@
+using Microsoft.VisualStudio.Shell;
+using System;
+
+namespace GitHub.Services
+{
+    public interface IVSUIContextFactory
+    {
+        IVSUIContext GetUIContext(Guid contextGuid);
+    }
+
+    public interface IVSUIContextChangedEventArgs
+    {
+        bool Activated { get; }
+    }
+
+    public interface IVSUIContext
+    {
+        bool IsActive { get; }
+        event EventHandler<IVSUIContextChangedEventArgs> UIContextChanged;
+    }
+}

src/GitHub.TeamFoundation.14/Base/TeamExplorerServiceHolder.cs

@@ -26,14 +26,24 @@ public class TeamExplorerServiceHolder : ITeamExplorerServiceHolder
         bool activeRepoNotified = false;
 
         IServiceProvider serviceProvider;
-        IGitExt gitService;
-        UIContext gitUIContext;
+        IVSGitExt gitService;
+        IVSUIContext gitUIContext;
+        IVSUIContextFactory uiContextFactory;
 
         // ActiveRepositories PropertyChanged event comes in on a non-main thread
         readonly SynchronizationContext syncContext;
 
-        public TeamExplorerServiceHolder()
+        /// <summary>
+        /// This class relies on IVSUIContextFactory to get the UIContext object that provides information
+        /// when VS switches repositories. Unfortunately, for some reason MEF fails to create the instance
+        /// when imported from the constructor, so it's imported manually when first accessed via the
+        /// ServiceProvider instance (when mocking, make sure that the ServiceProvider includes this factory)
+        /// </summary>
+        /// <param name="gitService"></param>
+        [ImportingConstructor]
+        public TeamExplorerServiceHolder(IVSGitExt gitService)
         {
+            this.GitService = gitService;
             syncContext = SynchronizationContext.Current;
         }
 
@@ -49,7 +59,7 @@ public IServiceProvider ServiceProvider
                 serviceProvider = value;
                 if (serviceProvider == null)
                     return;
-                GitUIContext = GitUIContext ?? UIContext.FromUIContextGuid(new Guid(Guids.GitSccProviderId));
+                GitUIContext = GitUIContext ?? UIContextFactory.GetUIContext(new Guid(Guids.GitSccProviderId));
                 UIContextChanged(GitUIContext?.IsActive ?? false, false);
             }
         }
@@ -77,7 +87,7 @@ public void Subscribe(object who, Action<ILocalRepositoryModel> handler)
 
             bool notificationsExist;
             ILocalRepositoryModel repo;
-            lock(activeRepoHandlers)
+            lock (activeRepoHandlers)
             {
                 repo = ActiveRepo;
                 notificationsExist = activeRepoNotified;
@@ -125,7 +135,7 @@ public void ClearServiceProvider(IServiceProvider provider)
 
         public void Refresh()
         {
-            GitUIContext = GitUIContext ?? UIContext.FromUIContextGuid(new Guid(Guids.GitSccProviderId));
+            GitUIContext = GitUIContext ?? UIContextFactory.GetUIContext(new Guid(Guids.GitSccProviderId));
             UIContextChanged(GitUIContext?.IsActive ?? false, true);
         }
 
@@ -139,14 +149,19 @@ void NotifyActiveRepo()
             }
         }
 
-        void UIContextChanged(object sender, UIContextChangedEventArgs e)
+        void UIContextChanged(object sender, IVSUIContextChangedEventArgs e)
         {
             Guard.ArgumentNotNull(e, nameof(e));
 
             ActiveRepo = null;
             UIContextChanged(e.Activated, false);
         }
 
+        /// <summary>
+        /// This is called on a background thread. Do not do synchronous GetService calls here.
+        /// </summary>
+        /// <param name="active"></param>
+        /// <param name="refresh"></param>
         async void UIContextChanged(bool active, bool refresh)
         {
             Debug.Assert(ServiceProvider != null, "UIContextChanged called before service provider is set");
@@ -155,42 +170,34 @@ async void UIContextChanged(bool active, bool refresh)
 
             if (active)
             {
-                GitService = GitService ?? ServiceProvider.GetServiceSafe<IGitExt>();
                 if (ActiveRepo == null || refresh)
+                {
                     ActiveRepo = await System.Threading.Tasks.Task.Run(() =>
                     {
-                        var repos = GitService?.ActiveRepositories;
+                        var repos = GitService.ActiveRepositories;
                         // Looks like this might return null after a while, for some unknown reason
                         // if it does, let's refresh the GitService instance in case something got wonky
                         // and try again. See issue #23
                         if (repos == null)
                         {
                             log.Error("Error 2001: ActiveRepositories is null. GitService: '{GitService}'", GitService);
-                            GitService = ServiceProvider?.GetServiceSafe<IGitExt>();
-                            repos = GitService?.ActiveRepositories;
+                            GitService.Refresh(ServiceProvider);
+                            repos = GitService.ActiveRepositories;
                             if (repos == null)
                                 log.Error("Error 2002: ActiveRepositories is null. GitService: '{GitService}'", GitService);
                         }
-                        return repos?.FirstOrDefault()?.ToModel();
+                        return repos?.FirstOrDefault();
                     });
+                }
             }
             else
                 ActiveRepo = null;
         }
 
-        void CheckAndUpdate(object sender, System.ComponentModel.PropertyChangedEventArgs e)
+        void UpdateActiveRepo()
         {
-            Guard.ArgumentNotNull(e, nameof(e));
-
-            if (e.PropertyName != "ActiveRepositories")
-                return;
-
-            var service = GitService;
-            if (service == null)
-                return;
-
-            var repo = service.ActiveRepositories.FirstOrDefault()?.ToModel();
-            if (repo != ActiveRepo)
+            var repo = gitService.ActiveRepositories.FirstOrDefault();
+            if (!Equals(repo, ActiveRepo))
                 // so annoying that this is on the wrong thread
                 syncContext.Post(r => ActiveRepo = r as ILocalRepositoryModel, repo);
         }
@@ -221,7 +228,7 @@ ITeamExplorerPage PageService
             get { return ServiceProvider.GetServiceSafe<ITeamExplorerPage>(); }
         }
 
-        UIContext GitUIContext
+        IVSUIContext GitUIContext
         {
             get { return gitUIContext; }
             set
@@ -236,18 +243,86 @@ UIContext GitUIContext
             }
         }
 
-        IGitExt GitService
+        IVSGitExt GitService
         {
             get { return gitService; }
             set
             {
                 if (gitService == value)
                     return;
                 if (gitService != null)
-                    gitService.PropertyChanged -= CheckAndUpdate;
+                    gitService.ActiveRepositoriesChanged -= UpdateActiveRepo;
                 gitService = value;
                 if (gitService != null)
-                    gitService.PropertyChanged += CheckAndUpdate;
+                    gitService.ActiveRepositoriesChanged += UpdateActiveRepo;
+            }
+        }
+
+        IVSUIContextFactory UIContextFactory
+        {
+            get
+            {
+                if (uiContextFactory == null)
+                {
+                    uiContextFactory = ServiceProvider.GetServiceSafe<IVSUIContextFactory>();
+                }
+                return uiContextFactory;
+            }
+        }
+    }
+
+    [Export(typeof(IVSUIContextFactory))]
+    [PartCreationPolicy(CreationPolicy.Shared)]
+    class VSUIContextFactory : IVSUIContextFactory
+    {
+        public IVSUIContext GetUIContext(Guid contextGuid)
+        {
+            return new VSUIContext(UIContext.FromUIContextGuid(contextGuid));
+        }
+    }
+
+    class VSUIContextChangedEventArgs : IVSUIContextChangedEventArgs
+    {
+        public bool Activated { get; }
+
+        public VSUIContextChangedEventArgs(bool activated)
+        {
+            Activated = activated;
+        }
+    }
+
+    class VSUIContext : IVSUIContext
+    {
+        readonly UIContext context;
+        readonly Dictionary<EventHandler<IVSUIContextChangedEventArgs>, EventHandler<UIContextChangedEventArgs>> handlers =
+            new Dictionary<EventHandler<IVSUIContextChangedEventArgs>, EventHandler<UIContextChangedEventArgs>>();
+        public VSUIContext(UIContext context)
+        {
+            this.context = context;
+        }
+
+        public bool IsActive { get { return context.IsActive; } }
+
+        public event EventHandler<IVSUIContextChangedEventArgs> UIContextChanged
+        {
+            add
+            {
+                EventHandler<UIContextChangedEventArgs> handler = null;
+                if (!handlers.TryGetValue(value, out handler))
+                {
+                    handler = (s, e) => value.Invoke(s, new VSUIContextChangedEventArgs(e.Activated));
+                    handlers.Add(value, handler);
+                }
+                context.UIContextChanged += handler;
+            }
+            remove
+            {
+                EventHandler<UIContextChangedEventArgs> handler = null;
+                if (handlers.TryGetValue(value, out handler))
+                {
+                    handlers.Remove(value);
+                    context.UIContextChanged -= handler;
+                }
             }
         }
     }

src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj

@@ -131,6 +131,7 @@
     <Reference Include="WindowsBase" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="Services\VSGitExt.cs" />
     <Compile Include="RegistryHelper.cs" />
     <Compile Include="Services\VSGitServices.cs" />
     <Compile Include="Settings.cs" />

src/GitHub.TeamFoundation.14/Services/VSGitExt.cs

@@ -0,0 +1,39 @@
+using System;
+using System.Collections.Generic;
+using System.ComponentModel.Composition;
+using System.Linq;
+using GitHub.Extensions;
+using GitHub.Models;
+using GitHub.Services;
+using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility;
+
+namespace GitHub.VisualStudio.Base
+{
+    [Export(typeof(IVSGitExt))]
+    [PartCreationPolicy(CreationPolicy.Shared)]
+    public class VSGitExt : IVSGitExt
+    {
+        IGitExt gitService;
+
+        public void Refresh(IServiceProvider serviceProvider)
+        {
+            if (gitService != null)
+                gitService.PropertyChanged -= CheckAndUpdate;
+            gitService = serviceProvider.GetServiceSafe<IGitExt>();
+            if (gitService != null)
+                gitService.PropertyChanged += CheckAndUpdate;
+        }
+
+
+        void CheckAndUpdate(object sender, System.ComponentModel.PropertyChangedEventArgs e)
+        {
+            Guard.ArgumentNotNull(e, nameof(e));
+            if (e.PropertyName != "ActiveRepositories" || gitService == null)
+                return;
+            ActiveRepositoriesChanged?.Invoke();
+        }
+
+        public IEnumerable<ILocalRepositoryModel> ActiveRepositories => gitService?.ActiveRepositories.Select(x => x.ToModel());
+        public event Action ActiveRepositoriesChanged;
+    }
+}

src/common/GitHubVS.ruleset

@@ -6,6 +6,7 @@
         <Rule Id="CA1002" Action="None" />
         <Rule Id="CA1004" Action="Warning" />
         <Rule Id="CA1006" Action="None" />
+        <Rule Id="CA1009" Action="None" />
         <Rule Id="CA1014" Action="None" />
         <Rule Id="CA1017" Action="None" />
         <Rule Id="CA1020" Action="None" />