diff --git a/src/GitHub.Exports/Services/IVSUIContextFactory.cs b/src/GitHub.Exports/Services/IVSUIContextFactory.cs index e3aca6cbb7..57c6542d9b 100644 --- a/src/GitHub.Exports/Services/IVSUIContextFactory.cs +++ b/src/GitHub.Exports/Services/IVSUIContextFactory.cs @@ -20,6 +20,6 @@ public VSUIContextChangedEventArgs(bool activated) public interface IVSUIContext { bool IsActive { get; } - event EventHandler UIContextChanged; + void WhenActivated(Action action); } } \ No newline at end of file diff --git a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs index 1573a10b7f..8b04edb866 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs @@ -1,5 +1,4 @@ using System; -using System.Windows; using System.Threading; using System.Runtime.InteropServices; using GitHub.Services; @@ -22,13 +21,9 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke { var usageTracker = (IUsageTracker)await GetServiceAsync(typeof(IUsageTracker)); var serviceProvider = (IGitHubServiceProvider)await GetServiceAsync(typeof(IGitHubServiceProvider)); + var gitExt = (IVSGitExt)await GetServiceAsync(typeof(IVSGitExt)); - // NOTE: ThreadingHelper.SwitchToMainThreadAsync() doesn't return until a solution is loaded. Using Dispatcher.Invoke instead. - Application.Current.Dispatcher.Invoke(() => - { - var gitExt = serviceProvider.GetService(); - new PullRequestStatusBarManager(gitExt, usageTracker, serviceProvider); - }); + new PullRequestStatusBarManager(gitExt, usageTracker, serviceProvider); } } } diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 6dd1d5fd12..5626ad4cd1 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using System.Threading.Tasks; using System.Collections.Generic; using System.ComponentModel.Composition; using GitHub.Models; @@ -8,21 +9,21 @@ using GitHub.TeamFoundation.Services; using Serilog; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; -using Task = System.Threading.Tasks.Task; namespace GitHub.VisualStudio.Base { /// /// This service acts as an always available version of . /// - [Export(typeof(IVSGitExt))] - [PartCreationPolicy(CreationPolicy.Shared)] + /// + /// Initialization for this service will be done asynchronously and the service will be + /// retrieved using . This means the service can be constructed and subscribed to from a background thread. + /// public class VSGitExt : IVSGitExt { static readonly ILogger log = LogManager.ForContext(); - readonly IGitHubServiceProvider serviceProvider; - readonly IVSUIContext context; + readonly Func> getServiceAsync; readonly ILocalRepositoryModelFactory repositoryFactory; readonly object refreshLock = new object(); @@ -30,55 +31,37 @@ public class VSGitExt : IVSGitExt IReadOnlyList activeRepositories; [ImportingConstructor] - public VSGitExt(IGitHubServiceProvider serviceProvider) - : this(serviceProvider, new VSUIContextFactory(), new LocalRepositoryModelFactory()) + public VSGitExt(Func> getServiceAsync) + : this(getServiceAsync, new VSUIContextFactory(), new LocalRepositoryModelFactory()) { } - public VSGitExt(IGitHubServiceProvider serviceProvider, IVSUIContextFactory factory, ILocalRepositoryModelFactory repositoryFactory) + public VSGitExt(Func> getServiceAsync, IVSUIContextFactory factory, ILocalRepositoryModelFactory repositoryFactory) { - this.serviceProvider = serviceProvider; + this.getServiceAsync = getServiceAsync; this.repositoryFactory = repositoryFactory; - // The IGitExt service isn't available when a TFS based solution is opened directly. - // It will become available when moving to a Git based solution (cause a UIContext event to fire). - context = factory.GetUIContext(new Guid(Guids.GitSccProviderId)); - - // Start with empty array until we have a change to initialize. + // Start with empty array until we have a chance to initialize. ActiveRepositories = Array.Empty(); - if (context.IsActive && TryInitialize()) - { - // Refresh ActiveRepositories on background thread so we don't delay startup. - InitializeTask = Task.Run(() => RefreshActiveRepositories()); - } - else - { - // If we're not in the UIContext or TryInitialize fails, have another go when the UIContext changes. - context.UIContextChanged += ContextChanged; - log.Debug("VSGitExt will be initialized later"); - InitializeTask = Task.CompletedTask; - } + // The IGitExt service isn't available when a TFS based solution is opened directly. + // It will become available when moving to a Git based solution (and cause a UIContext event to fire). + var context = factory.GetUIContext(new Guid(Guids.GitSccProviderId)); + context.WhenActivated(() => Initialize()); } - void ContextChanged(object sender, VSUIContextChangedEventArgs e) + void Initialize() { - // If we're in the UIContext and TryInitialize succeeds, we can stop listening for events. - // NOTE: this event can fire with UIContext=true in a TFS solution (not just Git). - if (e.Activated && TryInitialize()) + PendingTasks = getServiceAsync(typeof(IGitExt)).ContinueWith(t => { - // Refresh ActiveRepositories on background thread so we don't delay UI context change. - InitializeTask = Task.Run(() => RefreshActiveRepositories()); - context.UIContextChanged -= ContextChanged; - log.Debug("Initialized VSGitExt on UIContextChanged"); - } - } + gitService = (IGitExt)t.Result; + if (gitService == null) + { + log.Error("Couldn't find IGitExt service"); + return; + } - bool TryInitialize() - { - gitService = serviceProvider.GetService(); - if (gitService != null) - { + RefreshActiveRepositories(); gitService.PropertyChanged += (s, e) => { if (e.PropertyName == nameof(gitService.ActiveRepositories)) @@ -86,13 +69,7 @@ bool TryInitialize() RefreshActiveRepositories(); } }; - - log.Debug("Found IGitExt service and initialized VSGitExt"); - return true; - } - - log.Error("Couldn't find IGitExt service"); - return false; + }, TaskScheduler.Default); } void RefreshActiveRepositories() @@ -104,7 +81,7 @@ void RefreshActiveRepositories() log.Debug( "IGitExt.ActiveRepositories (#{Id}) returned {Repositories}", gitService.GetHashCode(), - gitService?.ActiveRepositories.Select(x => x.RepositoryPath)); + gitService.ActiveRepositories.Select(x => x.RepositoryPath)); ActiveRepositories = gitService?.ActiveRepositories.Select(x => repositoryFactory.Create(x.RepositoryPath)).ToList(); } @@ -136,6 +113,9 @@ private set public event Action ActiveRepositoriesChanged; - public Task InitializeTask { get; private set; } + /// + /// Tasks that are pending execution on the thread pool. + /// + public Task PendingTasks { get; private set; } = Task.CompletedTask; } } \ No newline at end of file diff --git a/src/GitHub.TeamFoundation.14/Services/VSUIContextFactory.cs b/src/GitHub.TeamFoundation.14/Services/VSUIContextFactory.cs index 6cedf85cd8..3797f75c02 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSUIContextFactory.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSUIContextFactory.cs @@ -1,6 +1,4 @@ using System; -using System.Collections.Generic; -using System.ComponentModel.Composition; using Microsoft.VisualStudio.Shell; using GitHub.Services; @@ -17,8 +15,7 @@ public IVSUIContext GetUIContext(Guid contextGuid) class VSUIContext : IVSUIContext { readonly UIContext context; - readonly Dictionary, EventHandler> handlers = - new Dictionary, EventHandler>(); + public VSUIContext(UIContext context) { this.context = context; @@ -26,27 +23,6 @@ public VSUIContext(UIContext 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; - } - } - } + public void WhenActivated(Action action) => context.WhenActivated(action); } } diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index 18f6d03a44..056d46301a 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -328,6 +328,7 @@ + @@ -679,6 +680,7 @@ True BuiltProjectOutputGroup;GetCopyToOutputDirectoryItems;DebugSymbolsProjectOutputGroup; DebugSymbolsProjectOutputGroup; + TF14 {161dbf01-1dbf-4b00-8551-c5c00f26720e} @@ -686,6 +688,7 @@ True BuiltProjectOutputGroup;GetCopyToOutputDirectoryItems;DebugSymbolsProjectOutputGroup; DebugSymbolsProjectOutputGroup; + TF15 {158b05e8-fdbc-4d71-b871-c96e28d5adf5} diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index bcd3c2271a..04d896c6aa 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -1,7 +1,6 @@ using System; using System.ComponentModel.Composition; using System.Diagnostics; -using System.Reflection; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -113,6 +112,7 @@ public GHClient(IProgram program) [ProvideService(typeof(IGitHubServiceProvider), IsAsyncQueryable = true)] [ProvideService(typeof(IUsageTracker), IsAsyncQueryable = true)] [ProvideService(typeof(IUsageService), IsAsyncQueryable = true)] + [ProvideService(typeof(IVSGitExt), IsAsyncQueryable = true)] [ProvideService(typeof(IGitHubToolWindowManager))] [Guid(ServiceProviderPackageId)] public sealed class ServiceProviderPackage : AsyncPackage, IServiceProviderPackage, IGitHubToolWindowManager @@ -150,6 +150,7 @@ Version VSVersion protected override Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { AddService(typeof(IGitHubServiceProvider), CreateService, true); + AddService(typeof(IVSGitExt), CreateService, true); AddService(typeof(IUsageTracker), CreateService, true); AddService(typeof(IUsageService), CreateService, true); AddService(typeof(ILoginManager), CreateService, true); @@ -258,6 +259,11 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT var serviceProvider = await GetServiceAsync(typeof(IGitHubServiceProvider)) as IGitHubServiceProvider; return new UsageTracker(serviceProvider, usageService); } + else if (serviceType == typeof(IVSGitExt)) + { + var vsVersion = ApplicationInfo.GetHostVersionInfo().FileMajorPart; + return VSGitExtFactory.Create(vsVersion, this); + } else if (serviceType == typeof(IGitHubToolWindowManager)) { return this; diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs new file mode 100644 index 0000000000..88bba6a1bb --- /dev/null +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -0,0 +1,47 @@ +extern alias TF14; +extern alias TF15; + +using System; +using System.ComponentModel.Composition; +using GitHub.Info; +using GitHub.Logging; +using Serilog; +using Microsoft.VisualStudio.Shell; +using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; +using VSGitExt15 = TF15.GitHub.VisualStudio.Base.VSGitExt; + +namespace GitHub.Services +{ + [PartCreationPolicy(CreationPolicy.Shared)] + public class VSGitExtFactory + { + static readonly ILogger log = LogManager.ForContext(); + + [ImportingConstructor] + public VSGitExtFactory(IGitHubServiceProvider serviceProvider) + { + VSGitExt = serviceProvider.GetService(); + } + + public static IVSGitExt Create(int vsVersion, IAsyncServiceProvider sp) + { + switch (vsVersion) + { + case 14: + return Create(() => new VSGitExt14(sp.GetServiceAsync)); + case 15: + return Create(() => new VSGitExt15(sp.GetServiceAsync)); + default: + log.Error("There is no IVSGitExt implementation for DTE version {Version}", vsVersion); + return null; + } + } + + // NOTE: We're being careful to only reference VSGitExt14 and VSGitExt15 from inside a lambda expression. + // This ensures that only the type that's compatible with the running DTE version is loaded. + static IVSGitExt Create(Func factory) => factory.Invoke(); + + [Export] + public IVSGitExt VSGitExt { get; } + } +} diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index 153d6295fc..daa0dbd037 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -11,22 +11,21 @@ using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; using System.Threading.Tasks; using System.Linq; -using GitHub.TeamFoundation.Services; public class VSGitExtTests { - public class TheConstructor + public class TheConstructor : TestBaseClass { [TestCase(true, 1)] [TestCase(false, 0)] public void GetServiceIGitExt_WhenSccProviderContextIsActive(bool isActive, int expectCalls) { var context = CreateVSUIContext(isActive); - var sp = Substitute.For(); + var sp = Substitute.For(); var target = CreateVSGitExt(context, sp: sp); - sp.Received(expectCalls).GetService(); + sp.Received(expectCalls).GetServiceAsync(typeof(IGitExt)); } [TestCase(true, 1)] @@ -34,13 +33,13 @@ public void GetServiceIGitExt_WhenSccProviderContextIsActive(bool isActive, int public void GetServiceIGitExt_WhenUIContextChanged(bool activated, int expectCalls) { var context = CreateVSUIContext(false); - var sp = Substitute.For(); + var sp = Substitute.For(); var target = CreateVSGitExt(context, sp: sp); - var eventArgs = new VSUIContextChangedEventArgs(activated); - context.UIContextChanged += Raise.Event>(context, eventArgs); + context.IsActive = activated; - sp.Received(expectCalls).GetService(); + target.PendingTasks.Wait(); + sp.Received(expectCalls).GetServiceAsync(typeof(IGitExt)); } [Test] @@ -60,10 +59,10 @@ public void ActiveRepositories_ReadUsingThreadPoolThread() } } - public class TheActiveRepositoriesChangedEvent + public class TheActiveRepositoriesChangedEvent : TestBaseClass { [Test] - public void GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() + public async Task GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() { var context = CreateVSUIContext(true); var gitExt = CreateGitExt(); @@ -75,6 +74,7 @@ public void GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() var eventArgs = new PropertyChangedEventArgs(nameof(gitExt.ActiveRepositories)); gitExt.PropertyChanged += Raise.Event(gitExt, eventArgs); + await target.PendingTasks; Assert.That(wasFired, Is.True); } @@ -106,9 +106,8 @@ public void WhenUIContextChanged_ActiveRepositoriesChangedIsFired() bool wasFired = false; target.ActiveRepositoriesChanged += () => wasFired = true; - var eventArgs = new VSUIContextChangedEventArgs(true); - context.UIContextChanged += Raise.Event>(context, eventArgs); - target.InitializeTask.Wait(); + context.IsActive = true; + target.PendingTasks.Wait(); Assert.That(wasFired, Is.True); } @@ -123,15 +122,14 @@ public void WhenUIContextChanged_FiredUsingThreadPoolThread() bool threadPool = false; target.ActiveRepositoriesChanged += () => threadPool = Thread.CurrentThread.IsThreadPoolThread; - var eventArgs = new VSUIContextChangedEventArgs(true); - context.UIContextChanged += Raise.Event>(context, eventArgs); - target.InitializeTask.Wait(); + context.IsActive = true; + target.PendingTasks.Wait(); Assert.That(threadPool, Is.True); } } - public class TheActiveRepositoriesProperty + public class TheActiveRepositoriesProperty : TestBaseClass { [Test] public void SccProviderContextNotActive_IsEmpty() @@ -150,7 +148,7 @@ public void SccProviderContextIsActive_InitializeWithActiveRepositories() var context = CreateVSUIContext(true); var gitExt = CreateGitExt(new[] { repoPath }); var target = CreateVSGitExt(context, gitExt, repoFactory: repoFactory); - target.InitializeTask.Wait(); + target.PendingTasks.Wait(); var activeRepositories = target.ActiveRepositories; @@ -167,7 +165,7 @@ public void ExceptionRefreshingRepositories_ReturnsEmptyList() var context = CreateVSUIContext(true); var gitExt = CreateGitExt(new[] { repoPath }); var target = CreateVSGitExt(context, gitExt, repoFactory: repoFactory); - target.InitializeTask.Wait(); + target.PendingTasks.Wait(); var activeRepositories = target.ActiveRepositories; @@ -188,6 +186,7 @@ public async Task ActiveRepositoriesChangedOrderingShouldBeCorrectAcrossThreads( var task2 = Task.Run(() => gitExt.ActiveRepositories = activeRepositories2); await Task.WhenAll(task1, task2); + await target.PendingTasks; Assert.That(target.ActiveRepositories.Single().LocalPath, Is.EqualTo("repo2")); } @@ -206,23 +205,27 @@ static IReadOnlyList CreateActiveRepositories(params string[ return repositories.AsReadOnly(); } - static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = null, IGitHubServiceProvider sp = null, + static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = null, IAsyncServiceProvider sp = null, ILocalRepositoryModelFactory repoFactory = null) { context = context ?? CreateVSUIContext(true); gitExt = gitExt ?? CreateGitExt(); - sp = sp ?? Substitute.For(); + sp = sp ?? Substitute.For(); repoFactory = repoFactory ?? Substitute.For(); var factory = Substitute.For(); var contextGuid = new Guid(Guids.GitSccProviderId); factory.GetUIContext(contextGuid).Returns(context); - sp.GetService().Returns(factory); - sp.GetService().Returns(gitExt); - var vsGitExt = new VSGitExt(sp, factory, repoFactory); - vsGitExt.InitializeTask.Wait(); + sp.GetServiceAsync(typeof(IGitExt)).Returns(gitExt); + var vsGitExt = new VSGitExt(sp.GetServiceAsync, factory, repoFactory); + vsGitExt.PendingTasks.Wait(); return vsGitExt; } + public interface IAsyncServiceProvider + { + Task GetServiceAsync(Type serviceType); + } + static IGitExt CreateGitExt(params string[] repositoryPaths) { var gitExt = Substitute.For(); @@ -231,11 +234,40 @@ static IGitExt CreateGitExt(params string[] repositoryPaths) return gitExt; } - static IVSUIContext CreateVSUIContext(bool isActive) + static MockVSUIContext CreateVSUIContext(bool isActive) + { + return new MockVSUIContext { IsActive = isActive }; + } + + class MockVSUIContext : IVSUIContext { - var context = Substitute.For(); - context.IsActive.Returns(isActive); - return context; + bool isActive; + Action action; + + public bool IsActive + { + get { return isActive; } + set + { + isActive = value; + if (isActive && action != null) + { + action.Invoke(); + action = null; + } + } + } + + public void WhenActivated(Action action) + { + if (isActive) + { + action.Invoke(); + return; + } + + this.action = action; + } } class MockGitExt : IGitExt