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