diff --git a/src/GitHub.Exports/GitHub.Exports.csproj b/src/GitHub.Exports/GitHub.Exports.csproj index cea2f9bf75..52f077afd8 100644 --- a/src/GitHub.Exports/GitHub.Exports.csproj +++ b/src/GitHub.Exports/GitHub.Exports.csproj @@ -265,6 +265,8 @@ + + diff --git a/src/GitHub.Exports/Services/IVSGitExt.cs b/src/GitHub.Exports/Services/IVSGitExt.cs new file mode 100644 index 0000000000..0b1be63c95 --- /dev/null +++ b/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 ActiveRepositories { get; } + event Action ActiveRepositoriesChanged; + } +} \ No newline at end of file diff --git a/src/GitHub.Exports/Services/IVSUIContext.cs b/src/GitHub.Exports/Services/IVSUIContext.cs new file mode 100644 index 0000000000..25acd9b632 --- /dev/null +++ b/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 UIContextChanged; + } +} \ No newline at end of file diff --git a/src/GitHub.TeamFoundation.14/Base/TeamExplorerServiceHolder.cs b/src/GitHub.TeamFoundation.14/Base/TeamExplorerServiceHolder.cs index e03bb5cbab..eb0912372c 100644 --- a/src/GitHub.TeamFoundation.14/Base/TeamExplorerServiceHolder.cs +++ b/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() + /// + /// 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) + /// + /// + [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 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,7 +149,7 @@ void NotifyActiveRepo() } } - void UIContextChanged(object sender, UIContextChangedEventArgs e) + void UIContextChanged(object sender, IVSUIContextChangedEventArgs e) { Guard.ArgumentNotNull(e, nameof(e)); @@ -147,6 +157,11 @@ void UIContextChanged(object sender, UIContextChangedEventArgs e) UIContextChanged(e.Activated, false); } + /// + /// This is called on a background thread. Do not do synchronous GetService calls here. + /// + /// + /// 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(); 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(); - 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(); } } - UIContext GitUIContext + IVSUIContext GitUIContext { get { return gitUIContext; } set @@ -236,7 +243,7 @@ UIContext GitUIContext } } - IGitExt GitService + IVSGitExt GitService { get { return gitService; } set @@ -244,10 +251,78 @@ IGitExt GitService 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(); + } + 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> handlers = + new Dictionary, EventHandler>(); + public VSUIContext(UIContext context) + { + this.context = context; + } + + public bool IsActive { get { return context.IsActive; } } + + public event EventHandler UIContextChanged + { + add + { + EventHandler 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 handler = null; + if (handlers.TryGetValue(value, out handler)) + { + handlers.Remove(value); + context.UIContextChanged -= handler; + } } } } diff --git a/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj b/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj index 821246ef44..90f158efde 100644 --- a/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj +++ b/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj @@ -131,6 +131,7 @@ + diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs new file mode 100644 index 0000000000..a0498bdfd2 --- /dev/null +++ b/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(); + 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 ActiveRepositories => gitService?.ActiveRepositories.Select(x => x.ToModel()); + public event Action ActiveRepositoriesChanged; + } +} \ No newline at end of file diff --git a/src/common/GitHubVS.ruleset b/src/common/GitHubVS.ruleset index d81ca51fac..63f2a176eb 100644 --- a/src/common/GitHubVS.ruleset +++ b/src/common/GitHubVS.ruleset @@ -6,6 +6,7 @@ +