Remove repository responsibilities from TeamExplorerServiceHolder (repository refactor part 2)

src/GitHub.App/SampleData/GitServiceDesigner.cs

@@ -10,7 +10,6 @@ class GitServiceDesigner : IGitService
     {
         public ILocalRepositoryModel CreateLocalRepositoryModel(string localPath) => null;
         public IBranch CreateCurrentBranchModel(ILocalRepositoryModel model) => null;
-        public void RefreshCloneUrl(ILocalRepositoryModel localRepositoryModel) { }
         public Task<string> GetLatestPushedSha(string path, string remote = "origin") => Task.FromResult<string>(null);
         public UriString GetRemoteUri(IRepository repo, string remote = "origin") => null;
         public IRepository GetRepository(string path) => null;

src/GitHub.Exports/Services/GitService.cs

@@ -68,18 +68,6 @@ public IBranch CreateCurrentBranchModel(ILocalRepositoryModel model)
             }
         }
 
-        /// <summary>
-        /// Updates the CloneUrl from the "origin" remote.
-        /// </summary>
-        public void RefreshCloneUrl(ILocalRepositoryModel localRepositoryModel)
-        {
-            var localPath = localRepositoryModel.LocalPath;
-            if (localPath == null)
-                return;
-
-            localRepositoryModel.CloneUrl = GetUri(localPath);
-        }
-
         /// <summary>
         /// Returns the URL of the remote for the specified <see cref="repository"/>. If the repository
         /// is null or no remote named origin exists, this method returns null

src/GitHub.Exports/Services/IGitService.cs

@@ -21,12 +21,6 @@ public interface IGitService
         /// <returns>A new branch model.</returns>
         IBranch CreateCurrentBranchModel(ILocalRepositoryModel model);
 
-        /// <summary>
-        /// Updates the CloneUrl information based on the "origin" remote.
-        /// </summary>
-        /// <param name="localRepositoryModel">The repository model to refresh.</param>
-        void RefreshCloneUrl(ILocalRepositoryModel localRepositoryModel);
-
         /// <summary>
         /// Returns the URL of the remote for the specified <see cref="repository"/>. If the repository
         /// is null or no remote exists, this method returns null

src/GitHub.Exports/Services/ITeamExplorerServiceHolder.cs

@@ -1,6 +1,7 @@
 using System;
 using GitHub.Primitives;
 using GitHub.Models;
+using Microsoft.VisualStudio.Threading;
 
 namespace GitHub.Services
 {
@@ -17,6 +18,7 @@ public interface ITeamExplorerServiceHolder
         /// changes in the source control context.
         /// </summary>
         IServiceProvider ServiceProvider { get; set; }
+
         /// <summary>
         /// Clears the current ServiceProvider if it matches the one that is passed in.
         /// This is usually called on Dispose, which might happen after another section
@@ -25,21 +27,16 @@ public interface ITeamExplorerServiceHolder
         /// </summary>
         /// <param name="provider">If the current ServiceProvider matches this, clear it</param>
         void ClearServiceProvider(IServiceProvider provider);
+
         /// <summary>
-        /// A IGitRepositoryInfo representing the currently active repository
-        /// </summary>
-        ILocalRepositoryModel ActiveRepo { get; }
-        /// <summary>
-        /// Subscribe to be notified when the active repository is set and Notify is called.
+        /// A service that can be used for repository changed events.
         /// </summary>
-        /// <param name="who">The instance that is interested in being called (or a unique key/object for that instance)</param>
-        /// <param name="handler">The handler to call when ActiveRepo is set</param>
-        void Subscribe(object who, Action<ILocalRepositoryModel> handler);
+        ITeamExplorerContext TeamExplorerContext { get; }
+
         /// <summary>
-        /// Unsubscribe from notifications
+        /// A service for avoiding deadlocks and marshaling tasks onto the UI thread.
         /// </summary>
-        /// <param name="who">The instance/key that previously subscribed to notifications</param>
-        void Unsubscribe(object who);
+        JoinableTaskFactory JoinableTaskFactory { get; }
 
         IGitAwareItem HomeSection { get; }
     }

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

@@ -1,6 +1,7 @@
 using System;
 using System.Diagnostics;
 using System.Drawing;
+using System.ComponentModel;
 using GitHub.Api;
 using GitHub.Extensions;
 using GitHub.Services;
@@ -14,6 +15,7 @@ namespace GitHub.VisualStudio.Base
 {
     public class TeamExplorerNavigationItemBase : TeamExplorerItemBase, ITeamExplorerNavigationItem2
     {
+        readonly ITeamExplorerServiceHolder holder;
         readonly Octicon octicon;
 
         public TeamExplorerNavigationItemBase(IGitHubServiceProvider serviceProvider,
@@ -24,6 +26,7 @@ public TeamExplorerNavigationItemBase(IGitHubServiceProvider serviceProvider,
             Guard.ArgumentNotNull(apiFactory, nameof(apiFactory));
             Guard.ArgumentNotNull(holder, nameof(holder));
 
+            this.holder = holder;
             this.octicon = octicon;
 
             IsVisible = false;
@@ -36,7 +39,31 @@ public TeamExplorerNavigationItemBase(IGitHubServiceProvider serviceProvider,
                 Invalidate();
             };
 
-            holder.Subscribe(this, UpdateRepo);
+            ActiveRepo = holder.TeamExplorerContext.ActiveRepository;
+            holder.TeamExplorerContext.PropertyChanged += TeamExplorerContext_PropertyChanged;
+            holder.TeamExplorerContext.StatusChanged += TeamExplorerContext_StatusChanged;
+        }
+
+        void TeamExplorerContext_StatusChanged(object sender, EventArgs e)
+        {
+            UpdateRepoOnMainThread(holder.TeamExplorerContext.ActiveRepository);
+        }
+
+        void TeamExplorerContext_PropertyChanged(object sender, PropertyChangedEventArgs e)
+        {
+            if (e.PropertyName == nameof(holder.TeamExplorerContext.ActiveRepository))
+            {
+                UpdateRepoOnMainThread(holder.TeamExplorerContext.ActiveRepository);
+            }
+        }
+
+        void UpdateRepoOnMainThread(ILocalRepositoryModel repo)
+        {
+            holder.JoinableTaskFactory.RunAsync(async () =>
+            {
+                await holder.JoinableTaskFactory.SwitchToMainThreadAsync();
+                UpdateRepo(repo);
+            }).Task.Forget();
         }
 
         public override async void Invalidate()
@@ -75,7 +102,8 @@ protected void OpenInBrowser(Lazy<IVisualStudioBrowser> browser, string endpoint
 
         void Unsubscribe()
         {
-            holder.Unsubscribe(this);
+            holder.TeamExplorerContext.PropertyChanged -= TeamExplorerContext_PropertyChanged;
+            holder.TeamExplorerContext.StatusChanged -= TeamExplorerContext_StatusChanged; ;
         }
 
         bool disposed;
@@ -109,7 +137,7 @@ public object Icon
         Image image;
         public Image Image
         {
-            get{ return image; }
+            get { return image; }
             set { image = value; this.RaisePropertyChange(); }
         }
     }

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

@@ -1,39 +1,26 @@
 using System;
-using System.Collections.Generic;
 using System.ComponentModel.Composition;
-using System.Linq;
 using GitHub.Extensions;
-using GitHub.Logging;
-using GitHub.Models;
 using GitHub.Services;
-using Serilog;
 using Microsoft.TeamFoundation.Controls;
 using Microsoft.VisualStudio.Shell;
 using Microsoft.VisualStudio.Threading;
-using System.Windows;
 
 namespace GitHub.VisualStudio.Base
 {
     [Export(typeof(ITeamExplorerServiceHolder))]
     [PartCreationPolicy(CreationPolicy.Shared)]
     public class TeamExplorerServiceHolder : ITeamExplorerServiceHolder
     {
-        static readonly ILogger log = LogManager.ForContext<TeamExplorerServiceHolder>();
-
-        readonly Dictionary<object, Action<ILocalRepositoryModel>> activeRepoHandlers = new Dictionary<object, Action<ILocalRepositoryModel>>();
-        ILocalRepositoryModel activeRepo;
-        bool activeRepoNotified = false;
-
         IServiceProvider serviceProvider;
-        readonly IVSGitExt gitExt;
-        readonly IGitService gitService;
 
         /// <summary>
         /// This class relies on IVSGitExt that provides information when VS switches repositories.
         /// </summary>
         /// <param name="gitExt">Used for monitoring the active repository.</param>
         [ImportingConstructor]
-        TeamExplorerServiceHolder(IVSGitExt gitExt, IGitService gitService) : this(gitExt, gitService, ThreadHelper.JoinableTaskContext)
+        TeamExplorerServiceHolder(ITeamExplorerContext teamExplorerContext) :
+            this(teamExplorerContext, ThreadHelper.JoinableTaskContext)
         {
         }
 
@@ -42,23 +29,13 @@ public class TeamExplorerServiceHolder : ITeamExplorerServiceHolder
         /// </summary>
         /// <param name="gitService">Used for monitoring the active repository.</param>
         /// <param name="joinableTaskContext">Used for switching to the Main thread.</param>
-        public TeamExplorerServiceHolder(IVSGitExt gitExt, IGitService gitService, JoinableTaskContext joinableTaskContext)
+        public TeamExplorerServiceHolder(ITeamExplorerContext teamExplorerContext, JoinableTaskContext joinableTaskContext)
         {
             JoinableTaskCollection = joinableTaskContext.CreateCollection();
             JoinableTaskCollection.DisplayName = nameof(TeamExplorerServiceHolder);
             JoinableTaskFactory = joinableTaskContext.CreateFactory(JoinableTaskCollection);
 
-            // HACK: This might be null in SafeMode.
-            // We should be throwing rather than returning a null MEF service.
-            if (gitExt == null)
-            {
-                return;
-            }
-
-            this.gitExt = gitExt;
-            this.gitService = gitService;
-            UpdateActiveRepo();
-            gitExt.ActiveRepositoriesChanged += UpdateActiveRepo;
+            TeamExplorerContext = teamExplorerContext;
         }
 
         // set by the sections when they get initialized
@@ -76,61 +53,6 @@ public IServiceProvider ServiceProvider
             }
         }
 
-        public ILocalRepositoryModel ActiveRepo
-        {
-            get { return activeRepo; }
-            private set
-            {
-                if (activeRepo == value)
-                    return;
-                if (activeRepo != null)
-                    activeRepo.PropertyChanged -= ActiveRepoPropertyChanged;
-                activeRepo = value;
-                if (activeRepo != null)
-                    activeRepo.PropertyChanged += ActiveRepoPropertyChanged;
-                NotifyActiveRepo();
-            }
-        }
-
-        public void Subscribe(object who, Action<ILocalRepositoryModel> handler)
-        {
-            Guard.ArgumentNotNull(who, nameof(who));
-            Guard.ArgumentNotNull(handler, nameof(handler));
-
-            bool notificationsExist;
-            ILocalRepositoryModel repo;
-            lock (activeRepoHandlers)
-            {
-                repo = ActiveRepo;
-                notificationsExist = activeRepoNotified;
-                if (!activeRepoHandlers.ContainsKey(who))
-                    activeRepoHandlers.Add(who, handler);
-                else
-                    activeRepoHandlers[who] = handler;
-            }
-
-            // the repo url might have changed and we don't get notifications
-            // for that, so this is a good place to refresh it in case that happened
-            if (repo != null)
-            {
-                gitService.RefreshCloneUrl(repo);
-            }
-
-            // if the active repo hasn't changed and there's notifications queued up,
-            // notify the subscriber. If the repo has changed, the set above will trigger
-            // notifications so we don't have to do it here.
-            if (repo == ActiveRepo && notificationsExist)
-                handler(repo);
-        }
-
-        public void Unsubscribe(object who)
-        {
-            Guard.ArgumentNotNull(who, nameof(who));
-
-            if (activeRepoHandlers.ContainsKey(who))
-                activeRepoHandlers.Remove(who);
-        }
-
         /// <summary>
         /// Clears the current ServiceProvider if it matches the one that is passed in.
         /// This is usually called on Dispose, which might happen after another section
@@ -148,39 +70,6 @@ public void ClearServiceProvider(IServiceProvider provider)
             ServiceProvider = null;
         }
 
-        void NotifyActiveRepo()
-        {
-            lock (activeRepoHandlers)
-            {
-                activeRepoNotified = true;
-                foreach (var handler in activeRepoHandlers.Values)
-                    handler(activeRepo);
-            }
-        }
-
-        void UpdateActiveRepo()
-        {
-            var repo = gitExt.ActiveRepositories.FirstOrDefault();
-
-            if (!Equals(repo, ActiveRepo))
-            {
-                // Fire property change events on Main thread
-                JoinableTaskFactory.RunAsync(async () =>
-                {
-                    await JoinableTaskFactory.SwitchToMainThreadAsync();
-                    ActiveRepo = repo;
-                }).Task.Forget(log);
-            }
-        }
-
-        void ActiveRepoPropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
-        {
-            Guard.ArgumentNotNull(e, nameof(e));
-
-            if (e.PropertyName == "CloneUrl")
-                ActiveRepo = sender as ILocalRepositoryModel;
-        }
-
         public IGitAwareItem HomeSection
         {
             get
@@ -200,6 +89,7 @@ ITeamExplorerPage PageService
         }
 
         public JoinableTaskCollection JoinableTaskCollection { get; }
-        JoinableTaskFactory JoinableTaskFactory { get; }
+        public JoinableTaskFactory JoinableTaskFactory { get; }
+        public ITeamExplorerContext TeamExplorerContext { get; }
     }
 }

src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs

@@ -28,6 +28,8 @@ namespace GitHub.VisualStudio.TeamExplorer.Connect
     public class GitHubConnectSection : TeamExplorerSectionBase, IGitHubConnectSection
     {
         static readonly ILogger log = LogManager.ForContext<GitHubConnectSection>();
+        readonly ISimpleApiClientFactory apiFactory;
+        readonly ITeamExplorerServiceHolder holder;
         readonly IPackageSettings packageSettings;
         readonly ITeamExplorerServices teamExplorerServices;
         readonly int sectionIndex;
@@ -101,8 +103,6 @@ public ILocalRepositoryModel SelectedRepository
 
         public ICommand Clone { get; }
 
-        internal ITeamExplorerServiceHolder Holder => holder;
-
         public GitHubConnectSection(IGitHubServiceProvider serviceProvider,
             ISimpleApiClientFactory apiFactory,
             ITeamExplorerServiceHolder holder,
@@ -127,6 +127,8 @@ public GitHubConnectSection(IGitHubServiceProvider serviceProvider,
             IsVisible = false;
             sectionIndex = index;
 
+            this.apiFactory = apiFactory;
+            this.holder = holder;
             this.packageSettings = packageSettings;
             this.teamExplorerServices = teamExplorerServices;
             this.localRepositories = localRepositories;
@@ -325,7 +327,7 @@ async void UpdateRepositoryList(object sender, NotifyCollectionChangedEventArgs
                     try
                     {
                         // TODO: Cache the icon state.
-                        var api = await ApiFactory.Create(newrepo.CloneUrl);
+                        var api = await apiFactory.Create(newrepo.CloneUrl);
                         var repo = await api.GetRepository();
                         newrepo.SetIcon(repo.Private, repo.Fork);
                     }
@@ -344,13 +346,13 @@ async void UpdateRepositoryList(object sender, NotifyCollectionChangedEventArgs
                         .Cast<ILocalRepositoryModel>()
                         .ForEach(async r =>
                     {
-                        if (Equals(Holder.ActiveRepo, r))
+                        if (Equals(holder.TeamExplorerContext.ActiveRepository, r))
                             SelectedRepository = r;
 
                         try
                         {
                             // TODO: Cache the icon state.
-                            var api = await ApiFactory.Create(r.CloneUrl);
+                            var api = await apiFactory.Create(r.CloneUrl);
                             var repo = await api.GetRepository();
                             r.SetIcon(repo.Private, repo.Fork);
                         }
@@ -457,7 +459,7 @@ public void Retry()
 
         public bool OpenRepository()
         {
-            var old = Repositories.FirstOrDefault(x => x.Equals(Holder.ActiveRepo));
+            var old = Repositories.FirstOrDefault(x => x.Equals(holder.TeamExplorerContext.ActiveRepository));
             if (!Equals(SelectedRepository, old))
             {
                 try